From 8111b3e73dec98dc28d06ff1abec792f1cc26d79 Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Tue, 14 Jan 2025 10:31:04 -0500 Subject: [PATCH 1/7] HDS-4327 Implement a11y toggle pattern in `hds-tooltip` --- .../components/src/modifiers/hds-tooltip.ts | 27 +++++++++++++++++-- .../hds/tooltip/tooltip-button-test.js | 14 +++++++--- .../hds/tooltip/tooltip-modifier-test.js | 11 ++++++-- 3 files changed, 45 insertions(+), 7 deletions(-) diff --git a/packages/components/src/modifiers/hds-tooltip.ts b/packages/components/src/modifiers/hds-tooltip.ts index 0ce4c142931..e95e71495bd 100644 --- a/packages/components/src/modifiers/hds-tooltip.ts +++ b/packages/components/src/modifiers/hds-tooltip.ts @@ -11,6 +11,7 @@ import type { ArgsFor } from 'ember-modifier'; import { assert } from '@ember/debug'; import { registerDestructor } from '@ember/destroyable'; +import { guidFor } from '@ember/object/internals'; import tippy, { followCursor } from 'tippy.js'; import type { @@ -32,10 +33,13 @@ export interface HdsTooltipModifierSignature { } function cleanup(instance: HdsTooltipModifier): void { - const { _interval, _needsTabIndex, _tooltip } = instance; + const { _interval, _needsTabIndex, _tooltip, _containerElement } = instance; if (_needsTabIndex) { _tooltip?.reference?.removeAttribute('tabindex'); } + if (_containerElement) { + _containerElement.remove(); + } clearInterval(_interval); _tooltip?.destroy(); } @@ -55,9 +59,11 @@ function cleanup(instance: HdsTooltipModifier): void { */ export default class HdsTooltipModifier extends Modifier { private _didSetup = false; + private _containerId: string = 'container-' + guidFor(this); _interval: number | undefined = undefined; _needsTabIndex = false; _tooltip: TippyInstance | undefined = undefined; + _containerElement?: HTMLElement; constructor(owner: unknown, args: ArgsFor) { super(owner, args); @@ -105,6 +111,7 @@ export default class HdsTooltipModifier extends Modifier`, // keeps tooltip itself open on hover: interactive: true, + appendTo: this._containerElement, // fix accessibility features that get messed up with setting interactive: true aria: { - content: 'describedby', expanded: false, }, content: () => content, diff --git a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js index b20ed4577f8..39a76984c76 100644 --- a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js +++ b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js @@ -81,19 +81,27 @@ module('Integration | Component | hds/tooltip/index', function (hooks) { assert.dom('.tippy-box').hasAttribute('role', 'tooltip'); }); - test('the button has an aria-describedby attribute with a value matching the tooltip id', async function (assert) { + test('the button has an aria-describedby and aria-controls attribute with a value matching the tooltip container', async function (assert) { await render( hbs`info` ); await focus('[data-test-tooltip-button]'); assert.dom('[data-test-tooltip-button]').hasAttribute('aria-describedby'); - assert.dom('[data-tippy-root]').hasAttribute('id'); + assert.dom('[data-test-tooltip-button]').hasAttribute('aria-controls'); + assert.dom('.hds-tooltip-container').hasAttribute('id'); assert.strictEqual( this.element .querySelector('[data-test-tooltip-button]') .getAttribute('aria-describedby'), - this.element.querySelector('[data-tippy-root]').getAttribute('id') + this.element.querySelector('.hds-tooltip-container').getAttribute('id') + ); + + assert.strictEqual( + this.element + .querySelector('[data-test-tooltip-button]') + .getAttribute('aria-controls'), + this.element.querySelector('.hds-tooltip-container').getAttribute('id') ); }); diff --git a/showcase/tests/integration/components/hds/tooltip/tooltip-modifier-test.js b/showcase/tests/integration/components/hds/tooltip/tooltip-modifier-test.js index df3f120cb6f..9ed957eb649 100644 --- a/showcase/tests/integration/components/hds/tooltip/tooltip-modifier-test.js +++ b/showcase/tests/integration/components/hds/tooltip/tooltip-modifier-test.js @@ -22,12 +22,19 @@ module('Integration | Component | hds/tooltip/index', function (hooks) { // test the expected accessibility related attributes: assert.dom('#test-tooltip-modifier').hasAttribute('aria-describedby'); - assert.dom('[data-tippy-root]').hasAttribute('id'); + assert.dom('.hds-tooltip-container').exists(); + assert.dom('.hds-tooltip-container').hasAttribute('id'); assert.strictEqual( this.element .querySelector('#test-tooltip-modifier') .getAttribute('aria-describedby'), - this.element.querySelector('[data-tippy-root]').getAttribute('id') + this.element.querySelector('.hds-tooltip-container').getAttribute('id') + ); + assert.strictEqual( + this.element + .querySelector('#test-tooltip-modifier') + .getAttribute('aria-controls'), + this.element.querySelector('.hds-tooltip-container').getAttribute('id') ); }); }); From 7f5ab9774441a6faefc642bafc4319f0619d04e4 Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Tue, 14 Jan 2025 10:35:03 -0500 Subject: [PATCH 2/7] Added changeset --- .changeset/dry-frogs-smash.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/dry-frogs-smash.md diff --git a/.changeset/dry-frogs-smash.md b/.changeset/dry-frogs-smash.md new file mode 100644 index 00000000000..367a267c47b --- /dev/null +++ b/.changeset/dry-frogs-smash.md @@ -0,0 +1,6 @@ +--- +"@hashicorp/design-system-components": patch +--- + +`hds-tooltip` - Changed DOM structure of tooltip content and set `aria-controls` on trigger elements for a11y improvements with toggled content +`TooltipButton` - Changed DOM structure of tooltip content and set `aria-controls` on button for a11y improvements with toggled content From db33872b9fabecac249e469d1677578406481a33 Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Thu, 16 Jan 2025 15:54:08 -0500 Subject: [PATCH 3/7] Fix: Failing tests in consumer repos, Styling bug in flexboxes --- packages/components/src/modifiers/hds-tooltip.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/components/src/modifiers/hds-tooltip.ts b/packages/components/src/modifiers/hds-tooltip.ts index e95e71495bd..81d7d66dfbe 100644 --- a/packages/components/src/modifiers/hds-tooltip.ts +++ b/packages/components/src/modifiers/hds-tooltip.ts @@ -131,14 +131,12 @@ export default class HdsTooltipModifier extends Modifier Date: Fri, 24 Jan 2025 15:44:28 -0500 Subject: [PATCH 4/7] Update changeset based on feedback --- .changeset/dry-frogs-smash.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.changeset/dry-frogs-smash.md b/.changeset/dry-frogs-smash.md index 367a267c47b..8b7b55bba7b 100644 --- a/.changeset/dry-frogs-smash.md +++ b/.changeset/dry-frogs-smash.md @@ -2,5 +2,6 @@ "@hashicorp/design-system-components": patch --- -`hds-tooltip` - Changed DOM structure of tooltip content and set `aria-controls` on trigger elements for a11y improvements with toggled content -`TooltipButton` - Changed DOM structure of tooltip content and set `aria-controls` on button for a11y improvements with toggled content +`hds-tooltip` - Changed DOM structure of tooltip content to add a wrapper around the tooltip content that is always in the DOM, and set `aria-controls` on trigger elements for a11y improvements with toggled content + +`TooltipButton` - Changed DOM structure of tooltip content to add a wrapper around the tooltip content that is always in the DOM, and set `aria-controls` on button for a11y improvements with toggled content From 83e1f780d4c4d6a5a98af094c20b479a1933a792 Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Mon, 3 Feb 2025 09:26:04 -0500 Subject: [PATCH 5/7] Fix: Clean up tooltip tests --- .../hds/tooltip/tooltip-button-test.js | 20 +++---------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js index 39a76984c76..e21caae790c 100644 --- a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js +++ b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js @@ -86,23 +86,9 @@ module('Integration | Component | hds/tooltip/index', function (hooks) { hbs`info` ); await focus('[data-test-tooltip-button]'); - assert.dom('[data-test-tooltip-button]').hasAttribute('aria-describedby'); - assert.dom('[data-test-tooltip-button]').hasAttribute('aria-controls'); - assert.dom('.hds-tooltip-container').hasAttribute('id'); - - assert.strictEqual( - this.element - .querySelector('[data-test-tooltip-button]') - .getAttribute('aria-describedby'), - this.element.querySelector('.hds-tooltip-container').getAttribute('id') - ); - - assert.strictEqual( - this.element - .querySelector('[data-test-tooltip-button]') - .getAttribute('aria-controls'), - this.element.querySelector('.hds-tooltip-container').getAttribute('id') - ); + const tooltipContainerId = this.element.querySelector('.hds-tooltip-container').getAttribute('id'); + assert.dom('[data-test-tooltip-button]').hasAttribute('aria-describedby', tooltipContainerId); + assert.dom('[data-test-tooltip-button]').hasAttribute('aria-controls', tooltipContainerId); }); // PLACEMENT From 0cd7174e6df08cd0458016fcdb2fb1d5bd6ddd37 Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Mon, 3 Feb 2025 09:29:48 -0500 Subject: [PATCH 6/7] Fix: Tooltip test linting --- .../components/hds/tooltip/tooltip-button-test.js | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js index e21caae790c..61443a7aac5 100644 --- a/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js +++ b/showcase/tests/integration/components/hds/tooltip/tooltip-button-test.js @@ -86,9 +86,15 @@ module('Integration | Component | hds/tooltip/index', function (hooks) { hbs`info` ); await focus('[data-test-tooltip-button]'); - const tooltipContainerId = this.element.querySelector('.hds-tooltip-container').getAttribute('id'); - assert.dom('[data-test-tooltip-button]').hasAttribute('aria-describedby', tooltipContainerId); - assert.dom('[data-test-tooltip-button]').hasAttribute('aria-controls', tooltipContainerId); + const tooltipContainerId = this.element + .querySelector('.hds-tooltip-container') + .getAttribute('id'); + assert + .dom('[data-test-tooltip-button]') + .hasAttribute('aria-describedby', tooltipContainerId); + assert + .dom('[data-test-tooltip-button]') + .hasAttribute('aria-controls', tooltipContainerId); }); // PLACEMENT From deb2a19f20de492b5817b4fa3ba70a6bcaf1099c Mon Sep 17 00:00:00 2001 From: Dylan Hyun Date: Mon, 3 Feb 2025 16:59:46 -0500 Subject: [PATCH 7/7] Update changeset based on feedback --- .changeset/dry-frogs-smash.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/dry-frogs-smash.md b/.changeset/dry-frogs-smash.md index 8b7b55bba7b..f628d338682 100644 --- a/.changeset/dry-frogs-smash.md +++ b/.changeset/dry-frogs-smash.md @@ -2,6 +2,6 @@ "@hashicorp/design-system-components": patch --- -`hds-tooltip` - Changed DOM structure of tooltip content to add a wrapper around the tooltip content that is always in the DOM, and set `aria-controls` on trigger elements for a11y improvements with toggled content +`hds-tooltip` - Changed structure of tooltip content to add a wrapper that is always in the DOM and set `aria-controls` on trigger elements for a11y improvements with toggled content -`TooltipButton` - Changed DOM structure of tooltip content to add a wrapper around the tooltip content that is always in the DOM, and set `aria-controls` on button for a11y improvements with toggled content +`TooltipButton` - Changed structure of tooltip content to add a wrapper that is always in the DOM and set `aria-controls` on button for a11y improvements with toggled content