[Version 8] Handle Shadow DOM subtrees when checking for active element #458
Conversation
| const firstFocusableChild = focusableChildren[0] | ||
| const lastFocusableChild = focusableChildren[focusableChildren.length - 1] | ||
|
|
||
| const activeElement = getDeepActiveElement() |
There was a problem hiding this comment.
🔍 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.
There was a problem hiding this comment.
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!
| // Get the active element, accounting for Shadow DOM subtrees. | ||
| // Credit to Cory LaViska for this inmplementation | ||
| // @see: https://www.abeautifulsite.net/posts/finding-the-active-element-in-a-shadow-root/ | ||
| function getDeepActiveElement( |
There was a problem hiding this comment.
This function checks for shadow subtrees and returns the active element within, if it exists.
| @@ -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 | |||
There was a problem hiding this comment.
🔍 We probably want previouslyFocused to be shadow-aware, to account for possible custom elements in useerland.
mxmason
changed the title
|
Aaaand I made a mistake. Merging #455 caused the branch to be deleted. Could you reopen this? Sorryyyy… |
| @@ -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 | |||
There was a problem hiding this comment.
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.
| const firstFocusableChild = focusableChildren[0] | ||
| const lastFocusableChild = focusableChildren[focusableChildren.length - 1] | ||
|
|
||
| const activeElement = getDeepActiveElement() |
There was a problem hiding this comment.
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!
| document.activeElement as HTMLElement | ||
| ) | ||
| const firstFocusableChild = focusableChildren[0] | ||
| const lastFocusableChild = focusableChildren[focusableChildren.length - 1] |
There was a problem hiding this comment.
Side note, now that we no longer supper Internet Explorer, we could use .at(-1) here to shorten this line.
| event.preventDefault() | ||
| } | ||
| } | ||
|
|
||
| // Get the active element, accounting for Shadow DOM subtrees. | ||
| // Credit to Cory LaViska for this inmplementation |
| // Get the active element, accounting for Shadow DOM subtrees. | ||
| // Credit to Cory LaViska for this inmplementation | ||
| // @see: https://www.abeautifulsite.net/posts/finding-the-active-element-in-a-shadow-root/ | ||
| function getDeepActiveElement( |
There was a problem hiding this comment.
Nice little function. Short and sweet, I like it.
Summary
This PR adds further support for Shadow DOM and custom elements, following work in #397. Note: it depends on changes in #455, so that should merge first.
Without these changes, our
trapTabKeyfunction doesn't understand when the user's focus is in a shadow subtree, so the user could escape the modal when we don't want them to.Why?
document.activeElementis not aware of any shadow roots in the document. We have to check for shadow roots on the active element, then get the active element within that root. SeegetDeepActiveElement().