fix(tab-header): focus problems (#1729)

telekom · Apr 17, 2023 · d1ed031 · d1ed031
1 parent 5c325d9
commit d1ed031
Show file tree

Hide file tree

Showing 6 changed files with 73 additions and 24 deletions.
diff --git a/packages/components/src/components/tab-header/readme.md b/packages/components/src/components/tab-header/readme.md
@@ -7,14 +7,13 @@
 
 ## Properties
 
-| Property    | Attribute    | Description                                                                           | Type                 | Default     |
-| ----------- | ------------ | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
-| `autoFocus` | `auto-focus` | (optional) autoFocus                                                                  | `boolean`            | `undefined` |
-| `disabled`  | `disabled`   | True for a disabled Tabnavigation                                                     | `boolean`            | `false`     |
-| `selected`  | `selected`   | (optional) Whether the tab is selected                                                | `boolean`            | `undefined` |
-| `size`      | `size`       | (optional) size                                                                       | `"large" \| "small"` | `'small'`   |
-| `small`     | `small`      | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean`            | `false`     |
-| `styles`    | `styles`     | (optional) Injected CSS styles                                                        | `string`             | `undefined` |
+| Property   | Attribute  | Description                                                                           | Type                 | Default     |
+| ---------- | ---------- | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
+| `disabled` | `disabled` | True for a disabled Tabnavigation                                                     | `boolean`            | `false`     |
+| `selected` | `selected` | (optional) Whether the tab is selected                                                | `boolean`            | `undefined` |
+| `size`     | `size`     | (optional) size                                                                       | `"large" \| "small"` | `'small'`   |
+| `small`    | `small`    | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean`            | `false`     |
+| `styles`   | `styles`   | (optional) Injected CSS styles                                                        | `string`             | `undefined` |
 
 
 ## Events

diff --git a/packages/components/src/components/tab-header/tab-header.tsx b/packages/components/src/components/tab-header/tab-header.tsx
@@ -46,8 +46,6 @@ export class TabHeader {
   @Prop() small?: boolean = false;
   /** (optional) size  */
   @Prop() size?: 'small' | 'large' = 'small';
-  /** (optional) autoFocus  */
-  @Prop() autoFocus?: boolean;
   /** (optional) Whether the tab is selected */
   @Prop() selected?: boolean;
   /** (optional) Injected CSS styles */
@@ -72,12 +70,10 @@ export class TabHeader {
       return;
     }
     if (!this.disabled) {
-      if (newValue === true) {
+      if (newValue === true && this.tabsHaveFocus()) {
         // Having focus on the host element, and not on inner elements,
         // is required because screen readers.
-        if (this.autoFocus) {
-          this.hostElement.focus();
-        }
+        this.hostElement.focus();
       }
       this.updateSlottedIcon();
     }
@@ -98,6 +94,19 @@ export class TabHeader {
     }
   }
 
+  /**
+   * Whether current focused element is within parent `scale-tab-nav`.
+   * Only if `true`, we imperatively focus the selected element.
+   * @returns boolean
+   */
+  tabsHaveFocus() {
+    const tabs = this.hostElement.closest('.scale-tab-nav');
+    if (!tabs) {
+      return false;
+    }
+    return tabs.contains(document.activeElement);
+  }
+
   /**
    * Find slotted icons, and if any, add the `selected` attribute accordingly.
    */

diff --git a/packages/components/src/components/tab-nav/__snapshots__/tab-nav.spec.ts.snap b/packages/components/src/components/tab-nav/__snapshots__/tab-nav.spec.ts.snap
@@ -0,0 +1,12 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`TabNav should match snapshot 1`] = `
+<scale-tab-nav class="scale-tab-nav" role="tablist">
+  <mock:shadow-root>
+    <div class="tab-nav tab-nav--" part="tab-nav">
+      <slot name="tab"></slot>
+      <slot name="panel"></slot>
+    </div>
+  </mock:shadow-root>
+</scale-tab-nav>
+`;
diff --git a/packages/components/src/components/tab-nav/readme.md b/packages/components/src/components/tab-nav/readme.md
@@ -7,12 +7,11 @@
 
 ## Properties
 
-| Property    | Attribute    | Description                                                                           | Type                 | Default     |
-| ----------- | ------------ | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
-| `autoFocus` | `auto-focus` | (optional) autoFocus                                                                  | `boolean`            | `false`     |
-| `size`      | `size`       | (optional) size                                                                       | `"large" \| "small"` | `'small'`   |
-| `small`     | `small`      | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean`            | `false`     |
-| `styles`    | `styles`     | (optional) Injected CSS styles                                                        | `string`             | `undefined` |
+| Property | Attribute | Description                                                                           | Type                 | Default     |
+| -------- | --------- | ------------------------------------------------------------------------------------- | -------------------- | ----------- |
+| `size`   | `size`    | (optional) size                                                                       | `"large" \| "small"` | `'small'`   |
+| `small`  | `small`   | <span style="color:red">**[DEPRECATED]**</span> - size should replace small<br/><br/> | `boolean`            | `false`     |
+| `styles` | `styles`  | (optional) Injected CSS styles                                                        | `string`             | `undefined` |
 
 
 ----------------------------------------------

diff --git a/packages/components/src/components/tab-nav/tab-nav.spec.ts b/packages/components/src/components/tab-nav/tab-nav.spec.ts
@@ -0,0 +1,33 @@
+/**
+ * @license
+ * Scale https://github.com/telekom/scale
+ *
+ * Copyright (c) 2021 Egor Kirpichev and contributors, Deutsche Telekom AG
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at https://mozilla.org/MPL/2.0/.
+ */
+
+import { newSpecPage } from '@stencil/core/testing';
+import { TabNav } from './tab-nav';
+
+describe('TabNav', () => {
+  let page;
+  beforeEach(async () => {
+    page = await newSpecPage({
+      components: [TabNav],
+      html: `<scale-tab-nav></scale-tab-nav>`,
+    });
+  });
+
+  it('should match snapshot', async () => {
+    expect(page.root).toMatchSnapshot();
+  });
+
+  // This class seems silly but it's needed in order to make sure we query the element
+  // since we can never be certain the name of the element will be <scale-tab-nav>.
+  it('should have default scale-tab-nav class', async () => {
+    expect(page.root.classList.contains('scale-tab-nav')).toBe(true);
+  });
+});
diff --git a/packages/components/src/components/tab-nav/tab-nav.tsx b/packages/components/src/components/tab-nav/tab-nav.tsx
@@ -35,8 +35,6 @@ export class TabNav {
   @Prop() small?: boolean = false;
   /** (optional) size  */
   @Prop() size: 'small' | 'large' = 'small';
-  /** (optional) autoFocus  */
-  @Prop() autoFocus: boolean = false;
   /** (optional) Injected CSS styles */
   @Prop() styles?: string;
 
@@ -151,7 +149,6 @@ export class TabNav {
     tabs.forEach((tab) => {
       const panel = tab.nextElementSibling;
       tab.setAttribute('aria-controls', panel.id);
-      tab.setAttribute('auto-focus', this.autoFocus.toString());
       panel.setAttribute('aria-labelledby', tab.id);
     });
     this.selectTab(selectedTab);
@@ -190,7 +187,7 @@ export class TabNav {
 
   render() {
     return (
-      <Host>
+      <Host class="scale-tab-nav">
         {this.styles && <style>{this.styles}</style>}
 
         <div part={this.getBasePartMap()} class={this.getCssClassMap()}>