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

sbb-design-systems · Sep 3, 2024 · f0ec0b0 · f0ec0b0
1 parent b99117b
commit f0ec0b0
Show file tree

Hide file tree

Showing 12 changed files with 74 additions and 72 deletions.
diff --git a/src/elements/autocomplete-grid/autocomplete-grid-option/autocomplete-grid-option.scss b/src/elements/autocomplete-grid/autocomplete-grid-option/autocomplete-grid-option.scss
@@ -14,7 +14,7 @@
   display: block;
 }
 
-:host([active]) {
+:host([data-active]) {
   --sbb-focus-outline-offset: calc(-1 * var(--sbb-spacing-fixed-1x));
 }
 
@@ -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);

diff --git a/...ements/autocomplete-grid/autocomplete-grid-option/autocomplete-grid-option.visual.spec.ts b/...ements/autocomplete-grid/autocomplete-grid-option/autocomplete-grid-option.visual.spec.ts
@@ -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

diff --git a/src/elements/autocomplete-grid/autocomplete-grid-option/readme.md b/src/elements/autocomplete-grid/autocomplete-grid-option/readme.md
@@ -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>
 ```
 
@@ -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
 

diff --git a/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.spec.ts b/src/elements/autocomplete-grid/autocomplete-grid/autocomplete-grid.spec.ts
@@ -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');
@@ -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');
@@ -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' });
@@ -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');

diff --git a/src/elements/autocomplete/autocomplete.spec.ts b/src/elements/autocomplete/autocomplete.spec.ts
@@ -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');

diff --git a/src/elements/option/option/__snapshots__/option.snapshot.spec.snap.js b/src/elements/option/option/__snapshots__/option.snapshot.spec.snap.js
@@ -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"
@@ -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">
@@ -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
@@ -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>

diff --git a/src/elements/option/option/option-base-element.ts b/src/elements/option/option/option-base-element.ts
@@ -44,8 +44,20 @@ export abstract class SbbOptionBaseElement extends SbbDisabledMixin(
     return this.getAttribute('value') ?? '';
   }
 
-  /** Whether the option is currently active. */
-  @property({ reflect: true, type: Boolean }) public active?: boolean;
+  /**
+   * Whether the option is currently active.
+   * @internal
+   */
+  public set active(value: boolean) {
+    this.toggleAttribute('data-active', value);
+  }
+  /**
+   * TODO: Remove getter with next major version (keep setter or change to method).
+   * @internal
+   */
+  public get active(): string {
+    return this.getAttribute('value') ?? '';
+  }
 
   /** Whether the option is selected. */
   @property({ type: Boolean })

diff --git a/src/elements/option/option/option.scss b/src/elements/option/option/option.scss
@@ -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));
 }
 
@@ -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);

diff --git a/src/elements/option/option/option.snapshot.spec.ts b/src/elements/option/option/option.snapshot.spec.ts
@@ -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>
           `)
@@ -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');
 });
diff --git a/src/elements/option/option/option.visual.spec.ts b/src/elements/option/option/option.visual.spec.ts
@@ -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

diff --git a/src/elements/option/option/readme.md b/src/elements/option/option/readme.md
@@ -16,15 +16,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-option` is nested in a `sbb-optgroup` component, it inherits from the parent the `disabled` state.
 
 ```html
 <sbb-option value="value" selected>Option label</sbb-option>
 
-<sbb-option value="value" active>Option label</sbb-option>
-
 <sbb-option value="value" disabled>Option label</sbb-option>
 ```
 
@@ -58,13 +56,12 @@ If the label slot contains only a **text node**, it is possible to search for te
 
 ## 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