-
Notifications
You must be signed in to change notification settings - Fork 452
♿(frontend) add focus trap and enter key support to remove doc modal #1531
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?
Conversation
70956fb to
34261f1
Compare
improves a11y by enabling keyboard-triggered modal with proper focus trap Signed-off-by: Cyril <c.gromoff@gmail.com>
34261f1 to
771de1c
Compare
|
Size Change: +255 B (+0.01%) Total Size: 3.68 MB
|
| const handleKeyDown = useCallback( | ||
| (event: KeyboardEvent<HTMLElement>, option: DropdownMenuOption) => { | ||
| if (event.key === 'Enter' || event.key === ' ') { | ||
| event.preventDefault(); | ||
| event.stopPropagation(); | ||
| onOpenChange?.(false); | ||
| void option.callback?.(); | ||
| } | ||
| }, | ||
| [onOpenChange], | ||
| ); | ||
|
|
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.
You just created the hook useKeyboardAction, you should use it.
| const cancelButton = document.getElementById(CANCEL_BUTTON_ID); | ||
| if (cancelButton instanceof HTMLButtonElement) { |
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.
I think it is better to use ref instead of an id or className in React.
| useEffect(() => { | ||
| const cancelButton = document.getElementById(CANCEL_BUTTON_ID); | ||
| if (cancelButton instanceof HTMLButtonElement) { | ||
| cancelButton.focus(); | ||
| } | ||
| }, []); |
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.
I don't understand the purpose of this useEffect, I don't see any changed when I comment this snippet.
Purpose
Improve accessibility and keyboard interactions for remove modal component
issue : 1530
Proposal
ModalRemoveDocuseKeyboardActionhook for Enter/Space handlers