fix(sbb-tooltip): fix accessibility bugs

sbb-design-systems · Jan 25, 2024 · 76cb03f · 76cb03f
1 parent 35a0173
commit 76cb03f
Show file tree

Hide file tree

Showing 8 changed files with 117 additions and 57 deletions.
diff --git a/src/components/core/a11y/focus.ts b/src/components/core/a11y/focus.ts
@@ -26,7 +26,7 @@ export function getFocusableElements(
 
   function getFocusables(elements: HTMLElement[], filterFunc?: (el: HTMLElement) => boolean): void {
     for (const el of elements) {
-      if (filterFunc?.(el)) {
+      if (filterFunc && !filterFunc(el)) {
         continue;
       }
 
@@ -76,7 +76,18 @@ export function getFirstFocusableElement(
 export class FocusHandler {
   private _controller = new AbortController();
 
-  public trap(element: HTMLElement, filterFunc?: (el: HTMLElement) => boolean): void {
+  /**
+   * @param element in which the focus should be trapped.
+   * @param options.filter filter function which is applied during searching for focusable element. If an element is filtered, also child elements are filtered.
+   * @param options.postFilter filter function which is applied after collecting focusable elements.
+   */
+  public trap(
+    element: HTMLElement,
+    options?: {
+      filter?: (el: HTMLElement) => boolean;
+      postFilter?: (el: HTMLElement) => boolean;
+    },
+  ): void {
     element.addEventListener(
       'keydown',
       (event) => {
@@ -88,9 +99,16 @@ export class FocusHandler {
         const elementChildren: HTMLElement[] = Array.from(
           element.shadowRoot!.children || [],
         ) as HTMLElement[];
-        const focusableElements = getFocusableElements(elementChildren, { filterFunc });
-        const firstFocusable = focusableElements[0] as HTMLElement;
-        const lastFocusable = focusableElements[focusableElements.length - 1] as HTMLElement;
+        const focusableElements = getFocusableElements(elementChildren, {
+          filterFunc: options?.filter,
+        });
+        const filteredFocusableElements = focusableElements.filter(
+          options?.postFilter ?? (() => true),
+        );
+        const firstFocusable = filteredFocusableElements[0] as HTMLElement;
+        const lastFocusable = filteredFocusableElements[
+          filteredFocusableElements.length - 1
+        ] as HTMLElement;
 
         const [pivot, next] = event.shiftKey
           ? [firstFocusable, lastFocusable]

diff --git a/src/components/datepicker/datepicker-toggle/datepicker-toggle.spec.ts b/src/components/datepicker/datepicker-toggle/datepicker-toggle.spec.ts
@@ -31,7 +31,6 @@ describe('sbb-datepicker-toggle', () => {
         data-state="closed"
         hide-close-button=""
         id="sbb-tooltip-1"
-        role="tooltip"
       >
         <sbb-calendar></sbb-calendar>
       </sbb-tooltip>
@@ -69,7 +68,6 @@ describe('sbb-datepicker-toggle', () => {
               hide-close-button=""
               data-state="closed"
               id="sbb-tooltip-4"
-              role="tooltip"
             >
               <sbb-calendar></sbb-calendar>
             </sbb-tooltip>
@@ -108,7 +106,6 @@ describe('sbb-datepicker-toggle', () => {
             data-state="closed"
             id="sbb-tooltip-7"
             hide-close-button=""
-            role="tooltip"
           >
             <sbb-calendar></sbb-calendar>
           </sbb-tooltip>
@@ -146,7 +143,6 @@ describe('sbb-datepicker-toggle', () => {
             hide-close-button=""
             data-state="closed"
             id="sbb-tooltip-10"
-            role="tooltip"
           >
             <sbb-calendar wide=""></sbb-calendar>
           </sbb-tooltip>

diff --git a/src/components/navigation/navigation/navigation.ts b/src/components/navigation/navigation/navigation.ts
@@ -205,7 +205,7 @@ export class SbbNavigationElement extends UpdateScheduler(LitElement) {
   }
 
   private _trapFocusFilter = (el: HTMLElement): boolean => {
-    return el.nodeName === 'SBB-NAVIGATION-SECTION' && el.getAttribute('data-state') !== 'opened';
+    return el.nodeName !== 'SBB-NAVIGATION-SECTION' || el.getAttribute('data-state') === 'opened';
   };
 
   // In rare cases it can be that the animationEnd event is triggered twice.
@@ -215,7 +215,7 @@ export class SbbNavigationElement extends UpdateScheduler(LitElement) {
       this._state = 'opened';
       this._didOpen.emit();
       applyInertMechanism(this);
-      this._focusHandler.trap(this, this._trapFocusFilter);
+      this._focusHandler.trap(this, { filter: this._trapFocusFilter });
       this._attachWindowEvents();
       this._setNavigationFocus();
     } else if (event.animationName === 'close' && this._state === 'closing') {

diff --git a/src/components/tooltip/tooltip-trigger/tooltip-trigger.scss b/src/components/tooltip/tooltip-trigger/tooltip-trigger.scss
@@ -27,6 +27,10 @@
   }
 }
 
+:host([negative]) {
+  --sbb-focus-outline-color: var(--sbb-focus-outline-color-dark);
+}
+
 :host(:is(:active, [data-active])) {
   --sbb-tooltip-color: var(--sbb-color-anthracite-default);
 }

diff --git a/src/components/tooltip/tooltip/tooltip.scss b/src/components/tooltip/tooltip/tooltip.scss
@@ -15,18 +15,14 @@
   --sbb-tooltip-border-radius: var(--sbb-border-radius-8x);
   --sbb-tooltip-padding: var(--sbb-spacing-fixed-4x);
   --sbb-tooltip-background: var(--sbb-color-white-default);
+  --sbb-tooltip-color: var(--sbb-color-charcoal-default);
   --sbb-tooltip-animation-duration: var(--sbb-animation-duration-4x);
   --sbb-tooltip-animation-easing: ease-out;
   --sbb-tooltip-transform: translateY(var(--sbb-spacing-fixed-2x));
 
   // As the tooltip has always a white background, we have to fix the focus outline color
   // to default color for cases where the tooltip is used in a negative context.
   --sbb-focus-outline-color: var(--sbb-focus-outline-color-default);
-
-  // We use this rule to make the inner container element to appear as if it were a
-  // direct child of the host's parent element. This is useful because the host
-  // should be ignored when using CSS grid or similar layout techniques.
-  display: contents;
 }
 
 :host([disable-animation]) {
@@ -61,6 +57,7 @@
   inset: var(--sbb-tooltip-inset);
   pointer-events: none;
   z-index: var(--sbb-tooltip-z-index, var(--sbb-overlay-z-index));
+  color: var(--sbb-tooltip-color);
 }
 
 .sbb-tooltip {
@@ -79,6 +76,11 @@
   max-width: var(--sbb-tooltip-max-width);
   width: max-content;
   background-color: var(--sbb-tooltip-background);
+  outline: none;
+
+  &:focus-visible:not([data-focus-origin='mouse'], [data-focus-origin='touch']) {
+    @include sbb.focus-outline;
+  }
 
   :host([data-state]:not([data-state='closed'])) & {
     display: block;

diff --git a/src/components/tooltip/tooltip/tooltip.spec.ts b/src/components/tooltip/tooltip/tooltip.spec.ts
@@ -1,47 +1,15 @@
 import { expect, fixture } from '@open-wc/testing';
 import { html } from 'lit/static-html.js';
 
-import { i18nCloseTooltip } from '../../core/i18n';
 import './tooltip';
 
 describe('sbb-tooltip', () => {
   it('renders', async () => {
     const root = await fixture(html`<sbb-tooltip></sbb-tooltip>`);
 
     expect(root).dom.to.be.equal(
-      `
-        <sbb-tooltip data-state="closed" id="sbb-tooltip-1" role="tooltip">
-
-        </sbb-tooltip>
-      `,
-    );
-    expect(root).shadowDom.to.be.equal(
-      `
-        <div class="sbb-tooltip__container">
-          <div class="sbb-tooltip">
-            <div class="sbb-tooltip__content">
-              <span>
-                <slot>
-                  No content
-                </slot>
-              </span>
-              <span class="sbb-tooltip__close">
-                <sbb-button
-                  dir="ltr"
-                  aria-label="${i18nCloseTooltip.en}"
-                  icon-name="cross-small"
-                  role="button"
-                  sbb-tooltip-close=""
-                  size="m"
-                  type="button"
-                  tabindex="0"
-                  variant="secondary"
-                ></sbb-button>
-              </span>
-            </div>
-          </div>
-        </div>
-      `,
+      `<sbb-tooltip data-state="closed" id="sbb-tooltip-1"></sbb-tooltip>`,
     );
+    await expect(root).shadowDom.to.be.equalSnapshot();
   });
 });
diff --git a/src/components/tooltip/tooltip/tooltip.stories.ts b/src/components/tooltip/tooltip/tooltip.stories.ts
@@ -53,6 +53,10 @@ const hoverTrigger: InputType = {
   },
 };
 
+const hideCloseButton: InputType = {
+  control: { type: 'boolean' },
+};
+
 const openDelay: InputType = {
   control: {
     type: 'number',
@@ -73,13 +77,15 @@ const disableAnimation: InputType = {
 
 const defaultArgTypes: ArgTypes = {
   'hover-trigger': hoverTrigger,
+  'hide-close-button': hideCloseButton,
   'open-delay': openDelay,
   'close-delay': closeDelay,
   'disable-animation': disableAnimation,
 };
 
 const defaultArgs: Args = {
   'hover-trigger': false,
+  'hide-close-button': false,
   'open-delay': undefined,
   'close-delay': undefined,
   'disable-animation': isChromatic(),
@@ -115,6 +121,17 @@ const tooltip = (args: Args): TemplateResult => html`
   </sbb-tooltip>
 `;
 
+const simpleTooltip = (args: Args): TemplateResult => html`
+  <sbb-tooltip data-testid="tooltip" trigger="tooltip-trigger" ${sbbSpread(args)}>
+    Simple tooltip without any interactive content but a list.
+    <ul aria-label="Colors">
+      <li>Red</li>
+      <li>Green</li>
+      <li>Blue</li>
+    </ul>
+  </sbb-tooltip>
+`;
+
 const StartBelowTemplate = (args: Args): TemplateResult => html`
   ${tooltipTrigger({ 'inset-inline-start': '2rem' })} ${tooltip(args)}
 `;
@@ -155,6 +172,10 @@ const HoverTriggerTemplate = (args: Args): TemplateResult => html`
   ${tooltipTrigger({ 'inset-inline-start': '2rem' })} ${tooltip(args)}
 `;
 
+const WithoutCloseButtonTemplate = (args: Args): TemplateResult => html`
+  ${tooltipTrigger({ 'inset-inline-start': '2rem' })} ${simpleTooltip(args)}
+`;
+
 export const StartBelow: StoryObj = {
   render: StartBelowTemplate,
   argTypes: defaultArgTypes,
@@ -216,6 +237,27 @@ export const HoverTrigger: StoryObj = {
   play: isChromatic() ? playStoryHover : undefined,
 };
 
+export const WithoutCloseButton: StoryObj = {
+  render: WithoutCloseButtonTemplate,
+  argTypes: defaultArgTypes,
+  args: {
+    ...defaultArgs,
+    'hide-close-button': true,
+  },
+  play: isChromatic() ? playStory : undefined,
+};
+
+export const WithoutCloseButtonHover: StoryObj = {
+  render: WithoutCloseButtonTemplate,
+  argTypes: defaultArgTypes,
+  args: {
+    ...defaultArgs,
+    'hide-close-button': true,
+    'hover-trigger': true,
+  },
+  play: isChromatic() ? playStoryHover : undefined,
+};
+
 const meta: Meta = {
   decorators: [
     (story) => html`

diff --git a/src/components/tooltip/tooltip/tooltip.ts b/src/components/tooltip/tooltip/tooltip.ts
@@ -207,14 +207,14 @@ export class SbbTooltipElement extends LitElement {
     if (newValue !== oldValue) {
       this._tooltipController?.abort();
       this._windowEventsController?.abort();
-      this._configure(this.trigger);
+      this._configure();
     }
   }
 
   public override connectedCallback(): void {
     super.connectedCallback();
     // Validate trigger element and attach event listeners
-    this._configure(this.trigger);
+    this._configure();
     this._state = 'closed';
     tooltipsRef.add(this as SbbTooltipElement);
   }
@@ -223,6 +223,10 @@ export class SbbTooltipElement extends LitElement {
     if (changedProperties.has('trigger')) {
       this._removeTriggerClickListener(this.trigger, changedProperties.get('trigger'));
     }
+
+    if (changedProperties.has('hoverTrigger')) {
+      this._configure();
+    }
   }
 
   protected override firstUpdated(): void {
@@ -241,14 +245,14 @@ export class SbbTooltipElement extends LitElement {
   }
 
   // Check if the trigger is valid and attach click event listeners.
-  private _configure(trigger?: string | HTMLElement): void {
+  private _configure(): void {
     removeAriaOverlayTriggerAttributes(this._triggerElement);
 
-    if (!trigger) {
+    if (!this.trigger) {
       return;
     }
 
-    this._triggerElement = findReferencedElement(trigger);
+    this._triggerElement = findReferencedElement(this.trigger);
 
     if (!this._triggerElement) {
       return;
@@ -389,12 +393,17 @@ export class SbbTooltipElement extends LitElement {
       this._didOpen.emit();
       this.inert = false;
       this._setTooltipFocus();
-      this._focusHandler.trap(this);
+      this._focusHandler.trap(this, {
+        postFilter: (el) => el !== this._overlay,
+      });
       this._attachWindowEvents();
     } else if (event.animationName === 'close' && this._state === 'closing') {
       this._state = 'closed';
       this._overlay?.firstElementChild?.scrollTo(0, 0);
 
+      // In case the overlay was closed without a relatedTarget, reset tabindex.
+      this._overlay.removeAttribute('tabindex');
+
       const elementToFocus = this._nextFocusedElement || this._triggerElement;
 
       setModalityOnNextFocus(elementToFocus);
@@ -416,6 +425,27 @@ export class SbbTooltipElement extends LitElement {
     if (firstFocusable) {
       setModalityOnNextFocus(firstFocusable);
       firstFocusable.focus();
+    } else {
+      this._overlay.setAttribute('tabindex', '0');
+      setModalityOnNextFocus(this._overlay);
+
+      this._overlay.focus();
+      this._overlay.addEventListener(
+        'blur',
+        (event) => {
+          // When a blur occurs, we know that the tooltip has to be closed, because there are no interactive elements inside the tooltip.
+          // If related target is not set, it's probably closed by click, or if a tab / window was changed.
+          // The first case is handled by click trigger while we in the second case want to avoid closing the tooltip on window / tab change.
+          if (event.relatedTarget instanceof HTMLElement) {
+            this._overlay.removeAttribute('tabindex');
+            this._nextFocusedElement = event.relatedTarget;
+            this.close();
+          }
+        },
+        {
+          signal: this._tooltipController.signal,
+        },
+      );
     }
   }
 
@@ -464,14 +494,14 @@ export class SbbTooltipElement extends LitElement {
     `;
 
     setAttribute(this, 'data-position', this._alignment?.vertical);
-    setAttribute(this, 'role', 'tooltip');
     assignId(() => this._tooltipId)(this);
 
     return html`
       <div class="sbb-tooltip__container">
         <div
           @animationend=${(event: AnimationEvent) => this._onTooltipAnimationEnd(event)}
           class="sbb-tooltip"
+          role="tooltip"
           ${ref((el?: Element) => (this._overlay = el as HTMLDivElement))}
         >
           <div