Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

web: Improve Patterns selector UI #1123

Merged
merged 2 commits into from
Apr 2, 2024
Merged

web: Improve Patterns selector UI #1123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d21d42a
web: Improve Patterns selector UI
dgdavid Apr 1, 2024
faf88f4
web: please linters
dgdavid Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions web/src/assets/styles/blocks.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ ul[data-type="agama/list"] {

li {
border: 2px solid var(--color-gray-dark);
padding: var(--spacer-normal);
padding: var(--spacer-small);
text-align: start;
background: var(--color-gray-light);
margin-block-end: 0;
Expand Down Expand Up @@ -276,7 +276,6 @@ ul[data-type="agama/list"] {
// not belongs to a listbox or grid list ul.
li[aria-selected] {
background: var(--color-gray-dark);
font-weight: 700;

&:not(:last-child) {
border-bottom-color: white;
Expand Down Expand Up @@ -314,15 +313,33 @@ ul[data-type="agama/list"][role="grid"] {
div[role="gridcell"] {
display: flex;
align-items: center;
gap: var(--spacer-normal);
gap: var(--spacer-small);

& > input {
input {
--size: var(--fs-h2);
cursor: pointer;
block-size: var(--size);
inline-size: var(--size);

&[data-auto-selected] {
accent-color: white;
box-shadow: 0 0 1px;
}
}

& > div:first-child {
display: flex;
flex-direction: column;
align-items: center;
gap: var(--spacer-small);

span {
font-size: var(--fs-small);
font-weight: bold;
}
}

& > div {
& > div:last-child {
flex: 1;
}
}
Expand Down Expand Up @@ -386,6 +403,18 @@ ul[data-type="agama/list"][role="grid"] {
}
}

ul[data-items-type="agama/patterns"] {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NOTE: this change reveals that this part, the core/Selector styles, can be simplified / improved. Let's wait a bit until selectors are more defined (we're working in new storage selectors that might have impact in such simplification if finally we get rid of storage/device-utils/DeviceSelector.

div[role="gridcell"] {
& > div:first-child {
min-width: 65px;
}

& > div:last-child * {
margin-block-end: var(--spacer-small);
}
}
}

[role="dialog"] {
.sticky-top-0 {
position: sticky;
Expand Down
21 changes: 15 additions & 6 deletions web/src/components/core/Selector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

// @ts-check
import React from "react";
import { _ } from "~/i18n";
import { noop } from "~/utils";

/**
Expand Down Expand Up @@ -61,6 +62,7 @@ import { noop } from "~/utils";
* @param {function} props.renderOption=noop - Function used for rendering options.
* @param {string} [props.optionIdKey="id"] - Key used for retrieve options id.
* @param {Array<*>} [props.selectedIds=[]] - Identifiers for selected options.
* @param {function} props.autoSelectionCheck=noop - Function used to check if option should be marked as auto selected.
* @param {onSelectionChangeCallback} [props.onSelectionChange=noop] - Callback to be called when the selection changes.
* @param {function|undefined} [props.onOptionClick] - Callback to be called when the selection changes.
* @param {object} [props.props] - Other props sent to the internal selector <ul> component
Expand All @@ -72,6 +74,7 @@ const Selector = ({
renderOption = noop,
optionIdKey = "id",
selectedIds = [],
autoSelectionCheck = noop,
onSelectionChange = noop,
onOptionClick,
...props
Expand Down Expand Up @@ -99,6 +102,7 @@ const Selector = ({
const optionId = option[optionIdKey];
const optionHtmlId = `${id}-option-${optionId}`;
const isSelected = selectedIds.includes(optionId);
const isAutoSelected = isSelected && autoSelectionCheck(option);
const onClick = () => onOptionClicked(optionId);

return (
Expand All @@ -108,14 +112,19 @@ const Selector = ({
role="row"
onClick={onClick}
aria-selected={isSelected || undefined}
data-auto-selected={isAutoSelected || undefined}
>
<div role="gridcell">
<input
type={isMultiple ? "checkbox" : "radio"}
checked={isSelected}
onChange={onClick}
aria-labelledby={optionHtmlId}
/>
<div>
<input
type={isMultiple ? "checkbox" : "radio"}
checked={isSelected}
onChange={onClick}
aria-labelledby={optionHtmlId}
data-auto-selected={isAutoSelected || undefined}
/>
{isAutoSelected && <div><span>{_("auto selected")}</span></div>}
</div>
{renderOption(option)}
</div>
</li>
Expand Down
15 changes: 15 additions & 0 deletions web/src/components/core/Selector.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,21 @@ describe("Selector", () => {
expect(onChangeFn).toHaveBeenCalledWith([2]);
});

it("sets data-auto-selected attribute to selected options for which `autoSelectionCheck` returns true", () => {
plainRender(
// Forcing a not selected option (en_GB) as auto selected for checking that it is not.
<TestingSelector autoSelectionCheck={o => ["es_ES", "en_GB"].includes(o.id)} />
);
const spanishRow = screen.getByRole("row", { name: "Spanish - Spain auto selected" });
const englishRow = screen.getByRole("row", { name: "English - United Kingdom" });
const spanishOption = within(spanishRow).getByRole("radio");
const englishOption = within(englishRow).getByRole("radio");
expect(spanishRow).toHaveAttribute("data-auto-selected");
expect(spanishOption).toHaveAttribute("data-auto-selected");
expect(englishRow).not.toHaveAttribute("data-auto-selected");
expect(englishOption).not.toHaveAttribute("data-auto-selected");
});

describe("when set as single selector", () => {
it("renders a radio input for each option", () => {
plainRender(<TestingSelector />);
Expand Down
6 changes: 4 additions & 2 deletions web/src/components/software/PatternSelector.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,12 +135,12 @@ function PatternSelector({ patterns, onSelectionChanged = noop }) {
const groups = groupPatterns(visiblePatterns);

const renderPatternOption = (pattern) => (
<>
<div>
<div>
<b>{pattern.summary}</b>
</div>
<div>{pattern.description}</div>
</>
</div>
);

const selector = sortGroups(groups).map((groupName) => {
Expand All @@ -159,6 +159,8 @@ function PatternSelector({ patterns, onSelectionChanged = noop }) {
onOptionClick={onToggle}
optionIdKey="name"
selectedIds={selectedIds}
autoSelectionCheck={pattern => pattern.selectedBy === SelectedBy.AUTO}
data-items-type="agama/patterns"
/>
</Section>
);
Expand Down
Loading