-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
hds-tooltip
- Implement a11y toggle content pattern
#2648
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
In smoke testing this change in nomad, there is a failing test that occurs due to this proposed change. The failed test is related to the job status component which contains a Update: Making some code changes to the implementation made it so this failed test can be avoided. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great with VoiceOver! Just had one non-blocking note on the changelog.
4111ccc
to
36dc044
Compare
await render( | ||
hbs`<Hds::TooltipButton @text="Hello" data-test-tooltip-button>info</Hds::TooltipButton>` | ||
); | ||
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( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: I think you could do this with DOM assertions
const tooltipContainerId = this.element.querySelector('.hds-tooltip-container').getAttribute('id');
assert.dom('[data-test-tooltip-button]').hasAttribute('aria-describedby', tooltipContainerId);
this.element.querySelector('.hds-tooltip-container').getAttribute('id') | ||
); | ||
|
||
assert.strictEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the same with this one.
📌 Summary
This PR implements usage of the
aria-controls
attribute in thehds-tooltip
modifier to align show/hide behavior across the repo as follows in the initiative from HDS-3581.The
hds-tooltip
modifier is used in theTooltipButton
.🛠️ Detailed description
Currently in the
hds-tooltip
modifier, the toggled content does not follow the proper a11y structure for toggled content, or leveragearia-controls
.As per a11y guidance in HDS-3581, all togged content should follow a pattern leveraging
aria-controls
, and all toggled containers should not be removed from the DOM on each toggle. They should always be present, and only the content inside them is added and removed.Currently the
hds-tooltip
follows a pattern where the entire content block is added and removed on each toggle, and anyaria-
attributes are set to theid
of this removable block.In this PR a new
hds-tooltip-container
block is now added as a sibling of the tooltip button, and is present in the DOM at all times. The yielded content is removed or added inside this container based on the tooltip visibility.The
aria-describedby
andaria-controls
attributes of the tooltip element now reference thehds-tooltip-container
.Note: The tooltip library leveraged, tippy.js, does not support this DOM structure out of the box or with any properties, so the container element has been created manually, and the tippy propery
appendTo
has been used to attach the tooltip content to this element.📸 Screenshots
Before
Inactive
Active
After
Inactive
Active
🔗 External links
Jira ticket: HDS-4327
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.