Skip to content

Commit

Permalink
fix(option-base): use data-active instead of active attribute (#3055)
Browse files Browse the repository at this point in the history
  • Loading branch information
jeripeierSBB authored Sep 5, 2024
1 parent 702fb78 commit 92cf0ca
Show file tree
Hide file tree
Showing 15 changed files with 85 additions and 82 deletions.
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
17 changes: 7 additions & 10 deletions src/elements/autocomplete-grid/autocomplete-grid-option/readme.md
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 @@ export class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
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 @@ export class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
} else {
const lastActiveOption = filteredOptions[this._activeItemIndex];
if (lastActiveOption) {
lastActiveOption.active = false;
lastActiveOption.setActive(false);
}
}
this._activeItemIndex = next;
Expand All @@ -178,15 +178,15 @@ export class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
const nextElement: SbbAutocompleteGridOptionElement | SbbAutocompleteGridButtonElement =
elementsInRow[next];
if (nextElement instanceof SbbAutocompleteGridOptionElement) {
nextElement.active = true;
nextElement.setActive(true);
} 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 @@ export class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
(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

0 comments on commit 92cf0ca

Please sign in to comment.