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.

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

fix(option-base): use data-active instead of active attribute #3055

Merged
merged 3 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
display: block;
}

:host([active]) {
:host([data-active]) {
--sbb-focus-outline-offset: calc(-1 * var(--sbb-spacing-fixed-1x));
}

Expand All @@ -41,7 +41,7 @@
padding-inline: var(--sbb-option-padding-inline);
color: var(--sbb-option-color);

:host([active]) & {
:host([data-active]) & {
@include sbb.focus-outline;

border-radius: var(--sbb-option-border-radius);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe(`sbb-autocomplete-grid-option`, () => {
<sbb-autocomplete-grid-option
style=${styleMap(style)}
icon-name=${iconName || nothing}
?active=${active && i === 0}
?data-active=${active && i === 0}
?disabled=${disabled && i === 0}
value=${`Value ${i + 1}`}
>Value ${i + 1}</sbb-autocomplete-grid-option
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,13 @@ at the component start using the `iconName` property or via custom content using

Like the native `option`, the component has a `value` property.

The `selected`, `disabled` and `active` properties are connected to the self-named states.
The `selected` and `disabled` properties are connected to the self-named states.
When disabled, the selection via click is prevented.
If the `sbb-autocomplete-grid-option` is nested in a `sbb-autocomplete-grid-optgroup` component, it inherits from the parent the `disabled` state.

```html
<sbb-autocomplete-grid-option value="value" selected>Option label</sbb-autocomplete-grid-option>

<sbb-autocomplete-grid-option value="value" active>Option label</sbb-autocomplete-grid-option>

<sbb-autocomplete-grid-option value="value" disabled>Option label</sbb-autocomplete-grid-option>
```

Expand Down Expand Up @@ -86,13 +84,12 @@ which is needed to correctly set the `aria-activedescendant` on the related `inp

## Properties

| Name | Attribute | Privacy | Type | Default | Description |
| ---------- | ----------- | ------- | ---------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `active` | `active` | public | `boolean \| undefined` | | Whether the option is currently active. |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `selected` | `selected` | public | `boolean` | | Whether the option is selected. |
| `value` | `value` | public | `string` | | Value of the option. |
| Name | Attribute | Privacy | Type | Default | Description |
| ---------- | ----------- | ------- | --------------------- | ------- | -------------------------------------------------------------------------------------------------------------------------------- |
| `disabled` | `disabled` | public | `boolean` | `false` | Whether the component is disabled. |
| `iconName` | `icon-name` | public | `string \| undefined` | | The icon name we want to use, choose from the small icon variants from the ui-icons category from here https://icons.app.sbb.ch. |
| `selected` | `selected` | public | `boolean` | | Whether the option is selected. |
| `value` | `value` | public | `string` | | Value of the option. |

## Events

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,30 +208,30 @@ describe(`sbb-autocomplete-grid`, () => {
await sendKeys({ press: 'ArrowDown' });
await sendKeys({ press: 'ArrowDown' });
await waitForLitRender(element);
expect(optTwo).to.have.attribute('active');
expect(optTwo).to.have.attribute('data-active');
expect(buttonTwo).not.to.have.attribute('data-focus-visible');
expect(buttonThree).not.to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'option-2');

await sendKeys({ press: 'ArrowRight' });
await waitForLitRender(element);
expect(optTwo).not.to.have.attribute('active');
expect(optTwo).not.to.have.attribute('data-active');
expect(buttonTwo).to.have.attribute('data-focus-visible');
expect(buttonThree).not.to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'button-2');

await sendKeys({ press: 'ArrowRight' });
await waitForLitRender(element);
expect(optTwo).not.to.have.attribute('active');
expect(optTwo).not.to.have.attribute('data-active');
expect(buttonTwo).not.to.have.attribute('data-focus-visible');
expect(buttonThree).to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'button-3');

await sendKeys({ press: 'ArrowDown' });
await waitForLitRender(element);
expect(optOne).to.have.attribute('active');
expect(optOne).to.have.attribute('data-active');
expect(buttonOne).not.to.have.attribute('data-focus-visible');
expect(optTwo).not.to.have.attribute('active');
expect(optTwo).not.to.have.attribute('data-active');
expect(buttonTwo).not.to.have.attribute('data-focus-visible');
expect(buttonThree).not.to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'option-1');
Expand All @@ -253,17 +253,17 @@ describe(`sbb-autocomplete-grid`, () => {
await sendKeys({ press: 'ArrowDown' });
await sendKeys({ press: 'ArrowDown' });
await waitForLitRender(element);
expect(optOne).not.to.have.attribute('active');
expect(optOne).not.to.have.attribute('data-active');
expect(optOne).not.to.have.attribute('selected');
expect(optTwo).to.have.attribute('active');
expect(optTwo).to.have.attribute('data-active');
expect(optTwo).not.to.have.attribute('selected');
expect(input).to.have.attribute('aria-activedescendant', 'option-2');

await sendKeys({ press: 'Enter' });
await waitForCondition(() => didCloseEventSpy.events.length === 1);
expect(didCloseEventSpy.count).to.be.equal(1);

expect(optTwo).not.to.have.attribute('active');
expect(optTwo).not.to.have.attribute('data-active');
expect(optTwo).to.have.attribute('selected');
expect(optionSelectedEventSpy.count).to.be.equal(1);
expect(input).to.have.attribute('aria-expanded', 'false');
Expand All @@ -283,10 +283,10 @@ describe(`sbb-autocomplete-grid`, () => {

await sendKeys({ press: 'ArrowDown' });
await waitForLitRender(element);
expect(optOne).to.have.attribute('active');
expect(optOne).to.have.attribute('data-active');
expect(buttonOne).not.to.have.attribute('data-focus-visible');
await sendKeys({ press: 'ArrowRight' });
expect(optOne).not.to.have.attribute('active');
expect(optOne).not.to.have.attribute('data-active');
expect(buttonOne).to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'button-1');
await sendKeys({ press: 'Enter' });
Expand All @@ -296,7 +296,7 @@ describe(`sbb-autocomplete-grid`, () => {
await sendKeys({ press: 'ArrowDown' });
await sendKeys({ press: 'ArrowRight' });
await waitForLitRender(element);
expect(optOne).not.to.have.attribute('active');
expect(optOne).not.to.have.attribute('data-active');
expect(buttonOne).not.to.have.attribute('data-focus-visible');
expect(buttonTwo).to.have.attribute('data-focus-visible');
expect(input).to.have.attribute('aria-activedescendant', 'button-2');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@
return;
}
const nextActiveOption = filteredOptions[next];
nextActiveOption.active = true;
nextActiveOption.setActive(true);
this.triggerElement?.setAttribute('aria-activedescendant', nextActiveOption.id);
nextActiveOption.scrollIntoView({ block: 'nearest' });

Expand All @@ -153,7 +153,7 @@
} else {
const lastActiveOption = filteredOptions[this._activeItemIndex];
if (lastActiveOption) {
lastActiveOption.active = false;
lastActiveOption.setActive(false);
}
}
this._activeItemIndex = next;
Expand All @@ -178,15 +178,15 @@
const nextElement: SbbAutocompleteGridOptionElement | SbbAutocompleteGridButtonElement =
elementsInRow[next];
if (nextElement instanceof SbbAutocompleteGridOptionElement) {
nextElement.active = true;
nextElement.setActive(true);

Check warning on line 181 in src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.ts

View check run for this annotation

Codecov / codecov/patch

src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.ts#L181

Added line #L181 was not covered by tests
} else {
nextElement.toggleAttribute('data-focus-visible', true);
}

const lastActiveElement: SbbAutocompleteGridOptionElement | SbbAutocompleteGridButtonElement =
elementsInRow[this._activeColumnIndex];
if (lastActiveElement instanceof SbbAutocompleteGridOptionElement) {
lastActiveElement.active = false;
lastActiveElement.setActive(false);
} else {
lastActiveElement.toggleAttribute('data-focus-visible', false);
}
Expand All @@ -205,7 +205,7 @@
(opt) => !opt.disabled && !opt.hasAttribute('data-group-disabled'),
)[this._activeItemIndex];
if (activeElement) {
activeElement.active = false;
activeElement.setActive(false);
}
}
this._activeItemIndex = -1;
Expand Down
6 changes: 3 additions & 3 deletions src/elements/autocomplete/autocomplete.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,17 +150,17 @@ describe(`sbb-autocomplete`, () => {
await sendKeys({ press: 'ArrowDown' });
await sendKeys({ press: 'ArrowDown' });
await waitForLitRender(element);
expect(optOne).not.to.have.attribute('active');
expect(optOne).not.to.have.attribute('data-active');
expect(optOne).not.to.have.attribute('selected');
expect(optTwo).to.have.attribute('active');
expect(optTwo).to.have.attribute('data-active');
expect(optTwo).not.to.have.attribute('selected');
expect(input).to.have.attribute('aria-activedescendant', 'option-2');

await sendKeys({ press: 'Enter' });
await waitForCondition(() => didCloseEventSpy.events.length === 1);
expect(didCloseEventSpy.count).to.be.equal(1);

expect(optTwo).not.to.have.attribute('active');
expect(optTwo).not.to.have.attribute('data-active');
expect(optTwo).to.have.attribute('selected');
expect(optionSelectedEventSpy.count).to.be.equal(1);
expect(input).to.have.attribute('aria-expanded', 'false');
Expand Down
6 changes: 3 additions & 3 deletions src/elements/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,14 +108,14 @@ export class SbbAutocompleteElement extends SbbAutocompleteBaseElement {
// Get and activate the next active option
const next = getNextElementIndex(event, this._activeItemIndex, filteredOptions.length);
const nextActiveOption = filteredOptions[next];
nextActiveOption.active = true;
nextActiveOption.setActive(true);
this.triggerElement?.setAttribute('aria-activedescendant', nextActiveOption.id);
nextActiveOption.scrollIntoView({ block: 'nearest' });

// Reset the previous active option
const lastActiveOption = filteredOptions[this._activeItemIndex];
if (lastActiveOption) {
lastActiveOption.active = false;
lastActiveOption.setActive(false);
}

this._activeItemIndex = next;
Expand All @@ -125,7 +125,7 @@ export class SbbAutocompleteElement extends SbbAutocompleteBaseElement {
const activeElement = this.options[this._activeItemIndex];

if (activeElement) {
activeElement.active = false;
activeElement.setActive(false);
}
this._activeItemIndex = -1;
this.triggerElement?.removeAttribute('aria-activedescendant');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
/* @web/test-runner snapshot v1 */
export const snapshots = {};

snapshots["sbb-option autocomplete renders selected and active DOM"] =
snapshots["sbb-option autocomplete renders selected DOM"] =
`<sbb-option
active=""
aria-selected="true"
data-slot-names="unnamed"
data-variant="autocomplete"
Expand All @@ -15,9 +14,9 @@ snapshots["sbb-option autocomplete renders selected and active DOM"] =
Option 1
</sbb-option>
`;
/* end snapshot sbb-option autocomplete renders selected and active DOM */
/* end snapshot sbb-option autocomplete renders selected DOM */

snapshots["sbb-option autocomplete renders selected and active Shadow DOM"] =
snapshots["sbb-option autocomplete renders selected Shadow DOM"] =
`<div class="sbb-option__container">
<div class="sbb-option">
<span class="sbb-option__icon">
Expand All @@ -32,7 +31,7 @@ snapshots["sbb-option autocomplete renders selected and active Shadow DOM"] =
</div>
</div>
`;
/* end snapshot sbb-option autocomplete renders selected and active Shadow DOM */
/* end snapshot sbb-option autocomplete renders selected Shadow DOM */

snapshots["sbb-option autocomplete renders disabled DOM"] =
`<sbb-option
Expand Down Expand Up @@ -67,35 +66,35 @@ snapshots["sbb-option autocomplete renders disabled Shadow DOM"] =
`;
/* end snapshot sbb-option autocomplete renders disabled Shadow DOM */

snapshots["sbb-option selected active Chrome"] =
snapshots["sbb-option selected Chrome"] =
`<p>
{
"role": "WebArea",
"name": ""
}
</p>
`;
/* end snapshot sbb-option selected active Chrome */
/* end snapshot sbb-option selected Chrome */

snapshots["sbb-option disabled Chrome"] =
snapshots["sbb-option selected Firefox"] =
`<p>
{
"role": "WebArea",
"role": "document",
"name": ""
}
</p>
`;
/* end snapshot sbb-option disabled Chrome */
/* end snapshot sbb-option selected Firefox */

snapshots["sbb-option selected active Firefox"] =
snapshots["sbb-option disabled Chrome"] =
`<p>
{
"role": "document",
"role": "WebArea",
"name": ""
}
</p>
`;
/* end snapshot sbb-option selected active Firefox */
/* end snapshot sbb-option disabled Chrome */

snapshots["sbb-option disabled Firefox"] =
`<p>
Expand Down
15 changes: 14 additions & 1 deletion src/elements/option/option/option-base-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,12 @@ export abstract class SbbOptionBaseElement extends SbbDisabledMixin(
return this.getAttribute('value') ?? '';
}

/** Whether the option is currently active. */
/**
* Whether the option is currently active.
* TODO: remove with next major version.
* @deprecated
* @internal
*/
@property({ reflect: true, type: Boolean }) public active?: boolean;

/** Whether the option is selected. */
Expand Down Expand Up @@ -166,6 +171,14 @@ export abstract class SbbOptionBaseElement extends SbbDisabledMixin(
this._optionAttributeObserver.disconnect();
}

/**
* Whether the option is currently active.
* @internal
*/
public setActive(value: boolean): void {
this.toggleAttribute('data-active', value);
}

protected init(): void {
this.setAttributeFromParent();
this._optionAttributeObserver.observe(this, optionObserverConfig);
Expand Down
4 changes: 2 additions & 2 deletions src/elements/option/option/option.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
--sbb-focus-outline-color: var(--sbb-focus-outline-color-dark);
}

:host([active]) {
:host([data-active]) {
--sbb-focus-outline-offset: calc(-1 * var(--sbb-spacing-fixed-1x));
}

Expand Down Expand Up @@ -97,7 +97,7 @@
-webkit-tap-highlight-color: transparent;
-webkit-text-fill-color: var(--sbb-option-color);

:host([active]) & {
:host([data-active]) & {
@include sbb.focus-outline;

border-radius: var(--sbb-option-border-radius);
Expand Down
9 changes: 3 additions & 6 deletions src/elements/option/option/option.snapshot.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ describe(`sbb-option`, () => {
describe('autocomplete', () => {
let element: SbbOptionElement;

describe('renders selected and active', async () => {
describe('renders selected', async () => {
beforeEach(async () => {
element = (
await fixture(html`
<sbb-autocomplete origin="anchor">
<sbb-option value="1" selected active>Option 1</sbb-option>
<sbb-option value="1" selected>Option 1</sbb-option>
</sbb-autocomplete>
<div id="anchor"></div>
`)
Expand Down Expand Up @@ -55,10 +55,7 @@ describe(`sbb-option`, () => {
});
});

testA11yTreeSnapshot(
html`<sbb-option value="1" selected active></sbb-option>`,
'selected active',
);
testA11yTreeSnapshot(html`<sbb-option value="1" selected></sbb-option>`, 'selected');

testA11yTreeSnapshot(html`<sbb-option value="1" disabled></sbb-option>`, 'disabled');
});
2 changes: 1 addition & 1 deletion src/elements/option/option/option.visual.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ describe(`sbb-option`, () => {
<sbb-option
style=${styleMap(style)}
icon-name=${iconName || nothing}
?active=${active && i === 0}
?data-active=${active && i === 0}
?disabled=${disabled && i === 0}
value=${`Value ${i + 1}`}
>Value ${i + 1}</sbb-option
Expand Down
Loading
Loading