Skip to content

Commit

Permalink
web: Improve Patterns selector UI (#1123)
Browse files Browse the repository at this point in the history
In #1112, the _Patterns selector_ was changed to be more consistent with
the rest of the UI selection dialogs. However, the _core/Selector_
component used under the hood wasn't ready to handle auto selection. A
partial adaptation was made by allowing overriding how it behaves when
an option is clicked (see 61b0c0f), but the UI part is being addressed
here by

* Improving how the information is layout
* Letting the user know which patterns are auto selected
  - Using a different _accent-color_ for the checkbox.
  - Adding the "auto selected" text below the checkbox, since a
just a different checkbox style (whatever we choose) is not enough for
letting the user know that it has been auto selected pattern in a quick
way.
  • Loading branch information
dgdavid authored Apr 2, 2024
2 parents 61853f4 + faf88f4 commit 4be45d2
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 13 deletions.
39 changes: 34 additions & 5 deletions web/src/assets/styles/blocks.scss
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"] {
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
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
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
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

0 comments on commit 4be45d2

Please sign in to comment.