Prevent scrolling when a MenuItem is selected with the spacebar #6697
Conversation
Prevent scroll in clickElementOnKeyPressBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
| keys.some(key => e.key === key) && e.target.dispatchEvent(new MouseEvent("click", { ...e, view: undefined })); | ||
| return (e: React.KeyboardEvent) => { | ||
| if (keys.some(key => e.key === key)) { | ||
| e.preventDefault(); // Prevent spacebar from scrolling the page |
There was a problem hiding this comment.
this could possibly cause issues if a MenuItem has a text input inside it, I think. it's not really recommended, but I wouldn't be surprised if something like this exists in the wild:
https://codesandbox.io/p/devbox/distracted-cookies-sv5mlx
we should test / verify this behavior in the docs by adding an item which contains an <InputGroup> to the Menu docs example
one idea for preventing a regression here is to check if the keypress occurred in a text input, using another utility fn in this same module:
if (elementIsTextInput(e.target as HTMLElement)) {
// don't prevent default
}There was a problem hiding this comment.
Ah, I didn't consider someone could be nesting a text element in a MenuItem. I can add a check and docs example for that.
Though if that were the case today, it would be issuing a click on the MenuItem any time they hit the spacebar. I assume we'd want to keep that behavior to avoid any breaks?
There was a problem hiding this comment.
it would be issuing a click on the MenuItem any time they hit the spacebar
Yeah I suppose so, but they might be doing their own preventDefault to work around this. Agree that we should preserve the existing behavior as much as possible to avoid surprises
| return (e: React.KeyboardEvent) => { | ||
| if (keys.some(key => e.key === key)) { | ||
| // Prevent spacebar from scrolling the page unless we're in a text field | ||
| if (!elementIsTextInput(e.target as HTMLElement)) { |
There was a problem hiding this comment.
This will allow the space character to enter the TextInput, but will still click the element. That's a bit odd, but technically prevents a break here.
| @@ -39,6 +40,12 @@ export function MenuExample(props: ExampleProps) { | |||
| <MenuItem icon="new-object" text="New object" /> | |||
| <MenuItem icon="new-link" text="New link" /> | |||
| <MenuDivider /> | |||
| <MenuItem | |||
There was a problem hiding this comment.
We didn't have any menu examples that performed an action on click previously.
Add more Menu docs examplesBuild artifact links for this commit: documentation | landing | table | demoThis is an automated comment from the deploy-preview CircleCI job. |
This fixes a small bug that caused the page to scroll when a user selected a
<MenuItem>using the spacebar, which is visible on the Menu docs page.