Skip to content
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

[Version 8] Handle Shadow DOM subtrees when checking for active element #458

Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions src/a11y-dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export default class A11yDialog {

// Keep a reference to the currently focused element to be able to restore
// it later
this.previouslyFocused = document.activeElement as HTMLElement
this.previouslyFocused = getDeepActiveElement() as HTMLElement
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 We probably want previouslyFocused to be shadow-aware, to account for possible custom elements in useerland.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we call it getActiveElement()? The fact that it’s deep is an implementation detail and does not need to be mentioned in the name I think.

this.shown = true
this.$el.removeAttribute('aria-hidden')

Expand Down Expand Up @@ -307,30 +307,49 @@ function isFocusable(el: HTMLElement) {
*/
function trapTabKey(el: HTMLElement, event: KeyboardEvent) {
const focusableChildren = getFocusableChildren(el)
const focusedItemIndex = focusableChildren.indexOf(
document.activeElement as HTMLElement
)
const firstFocusableChild = focusableChildren[0]
const lastFocusableChild = focusableChildren[focusableChildren.length - 1]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note, now that we no longer supper Internet Explorer, we could use .at(-1) here to shorten this line.

https://caniuse.com/mdn-javascript_builtins_array_at


const activeElement = getDeepActiveElement()
Comment on lines +310 to +313
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 We don't need to look through our collection of focusableChildren to get an index; we can compare our activeElement to the first and last nodes to check if the user is at the beginning or end of the modal.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That’s a good point. I’m not sure this was ever a real performance issue but potentially this could be when there are hundreds or thousands of focusable elements. Checking just the first and last is nicer. Well done!


// If the SHIFT key is pressed while tabbing (moving backwards) and the
// currently focused item is the first one, move the focus to the last
// focusable item from the dialog element
if (event.shiftKey && focusedItemIndex === 0) {
focusableChildren[focusableChildren.length - 1].focus()
if (event.shiftKey && activeElement === firstFocusableChild) {
lastFocusableChild.focus()
event.preventDefault()
}

// If the SHIFT key is not pressed (moving forwards) and the currently focused
// item is the last one, move the focus to the first focusable item from the
// dialog element
else if (
!event.shiftKey &&
focusedItemIndex === focusableChildren.length - 1
) {
focusableChildren[0].focus()
else if (!event.shiftKey && activeElement === lastFocusableChild) {
firstFocusableChild.focus()
event.preventDefault()
}
}

// Get the active element, accounting for Shadow DOM subtrees.
// Credit to Cory LaViska for this inmplementation
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation*

// @see: https://www.abeautifulsite.net/posts/finding-the-active-element-in-a-shadow-root/
function getDeepActiveElement(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function checks for shadow subtrees and returns the active element within, if it exists.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice little function. Short and sweet, I like it.

root: Document | ShadowRoot = document
): Element | null {
const activeEl = root.activeElement

if (!activeEl) {
return null
}

// If there's a shadow root, recursively look for the active element within it
if (activeEl.shadowRoot) {
return getDeepActiveElement(activeEl.shadowRoot)
// If not, we can just return the active element
} else {
return activeEl
}
}

function instantiateDialogs() {
$$('[data-a11y-dialog]').forEach(el => new A11yDialog(el))
}
Expand Down