Item: improve keyboard interaction and semantics (#3718)

Co-authored-by: RasmusKjeldgaard <rkk@bankdata.dk>
kirbydesign · Nov 21, 2024 · 7fb4030 · 7fb4030
1 parent ab0e31b
commit 7fb4030
Show file tree

Hide file tree

Showing 5 changed files with 88 additions and 36 deletions.
diff --git a/apps/cookbook/src/app/showcase/item-showcase/item-showcase.component.ts b/apps/cookbook/src/app/showcase/item-showcase/item-showcase.component.ts
@@ -12,8 +12,9 @@ import {
 export class ItemShowcaseComponent {
   properties: ApiDescriptionProperty[] = [
     {
-      name: 'Selectable',
-      description: 'Make item selectable',
+      name: 'selectable',
+      description:
+        'If `true`, a native button tag will be rendered under the hood and the item will become interactive.',
       defaultValue: 'false',
       type: ['boolean'],
     },

diff --git a/libs/designsystem/item/src/item.component.html b/libs/designsystem/item/src/item.component.html
@@ -1,9 +1,8 @@
 <ion-item
   lines="none"
   [attr.disabled]="disabled"
-  [attr.button]="_isIonicButton ? true : null"
-  [attr.tabindex]="selectable ? null : 0"
-  detail="false"
+  [attr.button]="_renderAsButton ? true : null"
+  [detail]="false"
   (mousedown)="onMouseDown($event)"
 >
   <div class="outside" slot="start">

diff --git a/libs/designsystem/item/src/item.component.integration.spec.ts b/libs/designsystem/item/src/item.component.integration.spec.ts
@@ -1,30 +1,33 @@
 import { createHostFactory, SpectatorHost } from '@ngneat/spectator';
 
+import { CheckboxComponent } from '@kirbydesign/designsystem/checkbox';
+import { RadioModule } from '@kirbydesign/designsystem/radio';
+import { ToggleComponent } from '@kirbydesign/designsystem/toggle';
 import { DesignTokenHelper } from '@kirbydesign/designsystem/helpers';
-
 import { TestHelper } from '@kirbydesign/designsystem/testing';
 
 import { ItemComponent } from './item.component';
 import { LabelComponent } from '.';
 
 const { fontWeight } = DesignTokenHelper;
 
-describe('ItemComponent with LabelComponent', () => {
+describe('ItemComponent', () => {
   let ionItem: HTMLElement;
 
   let spectator: SpectatorHost<ItemComponent>;
   const createHost = createHostFactory({
     component: ItemComponent,
-    imports: [TestHelper.ionicModuleForTest],
+    imports: [TestHelper.ionicModuleForTest, CheckboxComponent, RadioModule, ToggleComponent],
     declarations: [LabelComponent],
   });
 
-  describe('selectable and selected', () => {
-    let labelElements: Element[];
+  describe('with kirby-label', () => {
+    describe('selectable and selected', () => {
+      let labelElements: Element[];
 
-    beforeEach(async () => {
-      spectator = createHost(
-        `
+      beforeEach(async () => {
+        spectator = createHost(
+          `
         <kirby-item selectable="true" selected="true">
           <kirby-label>
             <h3>Title</h3>
@@ -36,28 +39,46 @@ describe('ItemComponent with LabelComponent', () => {
           </kirby-label>
         </kirby-item>
         `
-      );
-      ionItem = spectator.queryHost('ion-label');
-      await TestHelper.whenReady(ionItem);
-      labelElements = spectator.queryAll(
-        'ion-item ion-label > :is(h1, h2, h3, h4, h5, h6, p, data)'
-      );
-    });
+        );
+        ionItem = spectator.queryHost('ion-label');
+        await TestHelper.whenReady(ionItem);
+        labelElements = spectator.queryAll(
+          'ion-item ion-label > :is(h1, h2, h3, h4, h5, h6, p, data)'
+        );
+      });
+
+      it('should render heading, data and paragraph elements with correct font-weight', () => {
+        labelElements
+          .filter((e) => !e.attributes.getNamedItem('detail'))
+          .forEach((e) => {
+            expect(e).toHaveComputedStyle({ 'font-weight': fontWeight('bold') });
+          });
+      });
 
-    it('should render general header, data and paragraph elements with correct font-weight', () => {
-      labelElements
-        .filter((e) => !e.attributes.getNamedItem('detail'))
-        .forEach((e) => {
-          expect(e).toHaveComputedStyle({ 'font-weight': fontWeight('bold') });
-        });
+      it('should render detail data and paragraph elements with correct font-weight', () => {
+        labelElements
+          .filter((e) => !!e.attributes.getNamedItem('detail'))
+          .forEach((e) => {
+            expect(e).toHaveComputedStyle({ 'font-weight': fontWeight('normal') });
+          });
+      });
     });
+  });
+
+  describe('when configured with selectable="true"', () => {
+    const nestedInteractiveElements = ['kirby-checkbox', 'kirby-radio', 'kirby-toggle'];
+    nestedInteractiveElements.forEach((element) => {
+      it(`should not render native button when there is a nested ${element}`, async () => {
+        spectator = createHost(`<kirby-item selectable="true">
+          <${element}></${element}>
+        </kirby-item>
+        `);
+        ionItem = spectator.queryHost('ion-item');
+        await TestHelper.whenReady(ionItem);
 
-    it('should render detail data and paragraph elements with correct font-weight', () => {
-      labelElements
-        .filter((e) => !!e.attributes.getNamedItem('detail'))
-        .forEach((e) => {
-          expect(e).toHaveComputedStyle({ 'font-weight': fontWeight('normal') });
-        });
+        const nativePart = ionItem.shadowRoot.querySelector('[part="native"]');
+        expect(nativePart.tagName).not.toEqual('BUTTON');
+      });
     });
   });
 });
diff --git a/libs/designsystem/item/src/item.component.spec.ts b/libs/designsystem/item/src/item.component.spec.ts
@@ -33,6 +33,14 @@ describe('ItemComponent', () => {
     });
   });
 
+  it('should not render a native button by default', async () => {
+    const ionItem = spectator.queryHost('ion-item');
+    await TestHelper.whenReady(ionItem);
+
+    const nativePart = ionItem.shadowRoot.querySelector('[part="native"]');
+    expect(nativePart.tagName).not.toEqual('BUTTON');
+  });
+
   describe('when configured with size', () => {
     describe('and size = xs', () => {
       it('should have correct item height and padding', () => {
@@ -228,4 +236,24 @@ describe('ItemComponent', () => {
       expect(spectator.element).toHaveComputedStyle({ 'pointer-events': 'none' });
     });
   });
+
+  describe('when configured with selectable', () => {
+    it('should not render a native button when selectable="false"', async () => {
+      spectator.setInput('selectable', false);
+      const ionItem = spectator.queryHost('ion-item');
+      await TestHelper.whenReady(ionItem);
+
+      const nativePart = ionItem.shadowRoot.querySelector('[part="native"]');
+      expect(nativePart.tagName).not.toEqual('BUTTON');
+    });
+
+    it('should render a native button when selectable="true"', async () => {
+      spectator.setInput('selectable', true);
+      const ionItem = spectator.queryHost('ion-item');
+      await TestHelper.whenReady(ionItem);
+
+      const nativePart = ionItem.shadowRoot.querySelector('[part="native"]');
+      expect(nativePart.tagName).toEqual('BUTTON');
+    });
+  });
 });
diff --git a/libs/designsystem/item/src/item.component.ts b/libs/designsystem/item/src/item.component.ts
@@ -9,6 +9,7 @@ import {
 
 import { CheckboxComponent } from '@kirbydesign/designsystem/checkbox';
 import { RadioComponent } from '@kirbydesign/designsystem/radio';
+import { ToggleComponent } from '@kirbydesign/designsystem/toggle';
 
 export enum ItemSize {
   XS = 'xs',
@@ -46,6 +47,8 @@ export class ItemComponent {
   private checkbox: ElementRef<HTMLElement>;
   @ContentChild(RadioComponent, { static: false, read: ElementRef })
   private radio: ElementRef<HTMLElement>;
+  @ContentChild(ToggleComponent, { static: false, read: ElementRef })
+  private toggle: ElementRef<HTMLElement>;
 
   // Prevent default when inside kirby-dropdown to avoid blurring dropdown:
   onMouseDown(event: MouseEvent) {
@@ -57,9 +60,9 @@ export class ItemComponent {
     }
   }
 
-  get _isIonicButton() {
-    // Ionic checks for slotted checkbox and radio
-    // and we shouldn't set the `button` prop in that scenario:
-    return this.selectable && !(this.checkbox || this.radio);
+  get _renderAsButton() {
+    // We shouldn't render item as a button if the item contains
+    // nested interactive, i.e. checkbox, radio or toggle:
+    return this.selectable && !(this.checkbox || this.radio || this.toggle);
   }
 }