fix(dialog, dialog-full-screen, sidebar): make overflowing content tabbale #7052
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tomdavies73
changed the title
tomdavies73
force-pushed
the
FE-6839
branch
3 times, most recently
from
c2ee628
to
0164a65
Compare
edleeks87
reviewed
Nov 4, 2024
|
|
||
| elements.push(...focusableElements); | ||
|
|
||
| const scrollableElements = Array.from( |
There was a problem hiding this comment.
comment: I don't think we need to have an array here as it will only ever return one or zero elements with how are modals are currently structured. We may also be able to use contains to check if the found element contains anything focusable
const scrollableElement = ref.querySelector(defaultScrollableSelectors);
if (!scrollableElement) {
return;
}
if (
scrollableElement.scrollHeight > scrollableElement.clientHeight &&
(window.getComputedStyle(scrollableElement).overflowY === "scroll" ||
window.getComputedStyle(scrollableElement).overflowY === "auto") &&
!focusableElements.some(el => scrollableElement.contains(el))
) {
scrollableElement.setAttribute("tabindex", "0");
}
There was a problem hiding this comment.
Happy for this to be non-blocking if you find any issues or prefer your way as as far as I can tell it works etc
| @@ -5,6 +5,9 @@ type CustomRefObject<T> = { | |||
| const defaultFocusableSelectors = | |||
| 'button:not([disabled]), [href], input:not([type="hidden"]):not([disabled]), select:not([disabled]), textarea:not([disabled]), [tabindex]'; | |||
|
|
|||
| const defaultScrollableSelectors = | |||
| 'div[data-role="sidebar-content"], div[data-role="dialog-content"], div[data-role="dialog-full-screen-content"]'; | |||
There was a problem hiding this comment.
suggestion (non-blocking): it might be worth using data-component here so we're consistent with the other DOM query based implementations we have
damienrobson-sage
previously approved these changes
Nov 7, 2024
edleeks87
previously approved these changes
Nov 7, 2024
…bbable When any of the components' content containers overflow and contain no interactive elements, the container will be added to the tabbing order and can be focused via keyboard navigation to ensure their contents are accessible fix #6999
tomdavies73
dismissed stale reviews from edleeks87 and damienrobson-sage
via
774db78
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fix #6999
Proposed behaviour
When any of the components' content containers overflow and contain no interactive elements, the container will be added to the tabbing order and can be focused via keyboard navigation to ensure their contents are accessible.
Current behaviour
Currently, some overflowing content is inaccessible due to being in an overflowing container with no interactive elements.
Checklist
d.tsfile added or updated if requiredQA
Additional context
Testing instructions