-
Notifications
You must be signed in to change notification settings - Fork 67
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
Updates Menu Backdrop behavior #2767
Conversation
🦋 Changeset detectedLatest commit: 94909bb The changes in this PR will be included in the next version bump. This PR includes changesets to release 83 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This reverts commit 95bf339.
Size Change: +457 B (+0.03%) Total Size: 1.48 MB
ℹ️ View Unchanged
|
packages/lib/src/getClosestFocusableElement/getClosestFocusableElement.tsx
Outdated
Show resolved
Hide resolved
packages/lib/src/getClosestFocusableElement/getClosestFocusableElement.tsx
Outdated
Show resolved
Hide resolved
<MenuItem>Item A</MenuItem> | ||
<MenuItem>Item B</MenuItem> | ||
</Menu> | ||
<Button data-testid="outside-button" onClick={otherButtonHandler}> |
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.
@@ -42,12 +73,28 @@ export function useBackdropClick( | |||
* 5. Then we call the callback (typically fires `closeMenu`, setting `isOpen = false`, and rerender the component) | |||
*/ | |||
|
|||
// TODO: Remove this in a major version |
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.
make a ticket?
|
||
const handleClose = useCallback( | ||
(event?: MouseEvent | React.MouseEvent) => { | ||
// TODO: move this logic to useBackdropClick |
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.
ticket?
@@ -199,6 +228,7 @@ export const Menu = React.forwardRef<HTMLDivElement, MenuProps>(function Menu( | |||
onEntered={handlePopoverOpen} | |||
data-testid={LGIDs.root} | |||
data-lgid={LGIDs.root} | |||
ref={popoverRef} |
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.
why the switch?
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.
b/c the popover ref should be on the popover element
I'll add to the changeset
Hooks:
useBackdropClick
to accept an options object. Retains (but deprecates) boolean-only functionality.options.allowPropagation
to allow or disallow the click event to bubble up to other elements.Lib
Adds
getClosestFocusableElement
.This function crawls up the DOM tree to find the closest focusable element to the provided element
Returns the element itself if it is focusable or if no focusable element is found, it returns the body element.
Menu: