-
Couldn't load subscription status.
- Fork 235
fix(link): added delegateFocus for Safari tab index issue in shadow root #5580
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
84d5083
cc59f61
90e14ee
eb66e41
164b068
4c7f174
85d15aa
4e3a479
c4e0daf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@spectrum-web-components/link': patch | ||
| --- | ||
|
|
||
| ** Fixed ** : Safari keyboard navigation compatibility for sp-link component by implementing an approach with shadow DOM delegatesFocus and explicit tabindex. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,6 +15,7 @@ import { Link } from '@spectrum-web-components/link'; | |
| import { elementUpdated, expect, fixture, html } from '@open-wc/testing'; | ||
| import { testForLitDevWarnings } from '../../../test/testing-helpers.js'; | ||
| import { spy } from 'sinon'; | ||
| import { isWebKit } from '@spectrum-web-components/shared'; | ||
|
|
||
| describe('Link', () => { | ||
| testForLitDevWarnings( | ||
|
|
@@ -76,4 +77,34 @@ describe('Link', () => { | |
| el.click(); | ||
| expect(clickSpy.callCount).to.equal(0); | ||
| }); | ||
|
|
||
| it('has proper Safari keyboard navigation support when running in WebKit', async () => { | ||
| const el = await fixture<Link>(html` | ||
| <sp-link href="test_url">WebKit Test Link</sp-link> | ||
| `); | ||
|
|
||
| await elementUpdated(el); | ||
|
|
||
| // Always verify basic configuration | ||
| expect(el.shadowRoot).to.not.be.null; | ||
| expect(el.shadowRoot?.delegatesFocus).to.be.true; | ||
|
|
||
| const anchor = el.shadowRoot?.querySelector( | ||
| '#anchor' | ||
| ) as HTMLAnchorElement; | ||
| expect(anchor.getAttribute('tabindex')).to.eq('0'); | ||
|
|
||
| // WebKit-specific enhanced tests | ||
| if (isWebKit()) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we can remove this check and run the assertions for all browsers... any reason we need to scope it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes there is an upstream issue around this. It is a known limitation of iOS/Safari with custom elements and shadow DOM. See WebKit bug reports and related issues. |
||
| // Verify that the anchor element is properly focusable in Safari | ||
| expect(anchor.tabIndex).to.be.greaterThan(-1); | ||
|
|
||
| // Verify that the link maintains proper ARIA attributes in Safari | ||
| expect(anchor.hasAttribute('href')).to.be.true; | ||
| expect(anchor.getAttribute('role')).to.not.equal('button'); | ||
| } | ||
|
|
||
| // Common verification for all browsers | ||
| await expect(el).to.be.accessible(); | ||
| }); | ||
| }); | ||
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.
We usually use spread
...SpectrumElement.shadowRootOptions, should we also do it here?