-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add a quick trigger action to the Menu
, Listbox
and Combobox
components
#3700
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
Conversation
For consistency
Scenario: 1. Mousedown on the MenuButton to open the menu 2. Hold down, and drag your mouse over a MenuItem 3. Release the mouse (mouseup/pointerup) on the MenuItem 4. MenuItem is "invoked"
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Menu
componentMenu
, Listbox
and Combobox
components
// Source: https://github.com/testing-library/react-testing-library/issues/838#issuecomment-735259406 | ||
// | ||
// Polyfill the PointerEvent class for JSDOM | ||
class PointerEvent extends Event { |
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.
Had to add this to make the tests pass
let enableQuickRelease = comboboxState === ComboboxState.Open | ||
useQuickRelease(enableQuickRelease, { | ||
trigger: localButtonElement, | ||
action: useCallback( | ||
(e) => { | ||
if (localButtonElement?.contains(e.target)) { | ||
return QuickReleaseAction.Ignore | ||
} | ||
|
||
if (inputElement?.contains(e.target)) { | ||
return QuickReleaseAction.Ignore | ||
} | ||
|
||
let option = e.target.closest('[role="option"]:not([data-disabled])') | ||
if (option !== null) { | ||
return QuickReleaseAction.Select(option as HTMLElement) | ||
} | ||
|
||
if (optionsElement?.contains(e.target)) { | ||
return QuickReleaseAction.Ignore | ||
} | ||
|
||
return QuickReleaseAction.Close | ||
}, | ||
[localButtonElement, inputElement, optionsElement] | ||
), | ||
close: machine.actions.closeCombobox, | ||
select: machine.actions.selectActiveOption, | ||
}) |
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.
Open to suggestions for the API here. But essentially what we want to know is:
- What is the trigger element where it all starts. E.g.:
MenuButton
- Depending on the current target when we release the
pointerup
, what action should we take? Options are: Ignore
— essentially do nothing. In this case it means keep theMenu
open and that's it.Select
— select the current DOM node. In theListbox
andCombobox
we can rely on theselectActiveOption
. In theMenu
we don't have that, so instead we just look for the[role="menuitem"]
. If it exists and it's not disabled then we can select this option.Close
— this should close the dropdown again. This will happen if you drag off theMenuButton
for example.- We need a way to close the current dropdown
- We need a way to select the current element. This will receive the element you provided via
Select(…)
.
The reason I didn't just call close()
and select()
as part of the action is because we also have to make sure that we take the ~100ms delay into account. This threshold is only relevant for if a dropdown opens on top of the current cursor position (e.g.: https://catalyst.tailwindui.com/docs/listbox) because then we don't want to select the item.
If we check for the time differences for any action, then it could result in the menu not closing if you were quick enough.
This PR adds a new quick trigger feature to the
Menu
. Not sure what the bestname for this is, but essentially this is the behavior:
Recently we made sure that the
Menu
opens onmousedown
(not justclick
).This means that we can perform the following quick action:
mousedown
on theMenuButton
— this will open theMenu
MenuItem
s — this will highlight the currently activeMenuItem
.MenuItem
and close theMenu
.This now means that you can perform actions very quickly.
What this PR doesn't do yet is if you have a scrollable list, then it won't scroll up or down when you reach the ends of the list. For this we would need to introduce some new elements. The native Menu items on macOS show a little placeholder arrow. If you put your cursor in that area, it starts scrolling:
Test plan