-
Notifications
You must be signed in to change notification settings - Fork 377
Better dropdowns #4769
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
Merged
Merged
Better dropdowns #4769
Changes from 34 commits
Commits
Show all changes
50 commits
Select commit
Hold shift + click to select a range
9f6ec87
Update package-lock.json
Gazook89 399f04d
Merge remote-tracking branch 'upstream/master'
Gazook89 dc8ff1c
Merge remote-tracking branch 'upstream/master'
Gazook89 b5cd7b7
package-lock change?
Gazook89 2232d52
Add basic nesting dropdown component
Gazook89 3b6c7ac
snippet submenu triggers don't fire events onclick
Gazook89 4f953da
caret menu indicators work, point correct direction
Gazook89 8bebbda
restore styling to work with new structure.
Gazook89 992b72e
empty menus styled as disabled
Gazook89 0c78a23
remove testing menu
Gazook89 904b14a
icons in triggers and menus are aligned.
Gazook89 285522d
all snippet buttons are actually buttons now
Gazook89 c0c4db9
remove id attribute from component
Gazook89 87ed5d1
add jsdoc documentation.
Gazook89 359d874
add oddbird polyfill - not quite working
Gazook89 d686c78
fix oddbird polyfill so custom properties are used correctly
Gazook89 4d3179f
remove unused less file
Gazook89 5b11827
fix disabled attribute
Gazook89 d368e53
add some comments
Gazook89 d79c4d9
Merge remote-tracking branch 'upstream/master'
Gazook89 612614f
menu items dismiss menu on click by default
Gazook89 2812f30
Merge remote-tracking branch 'upstream/master'
Gazook89 5e12262
Merge branch 'master' into Better-Dropdowns
Gazook89 2e4439c
Remove ID collisions on repeated menu names
Gazook89 3110d2c
Add icons to menu triggers
Gazook89 1319694
popovertarget is typically not camelcase in html
Gazook89 c12a188
Be sure snippet icon is getting passed in
Gazook89 af0804f
Remove non-existent icons
Gazook89 bb3f6fe
remove nonexistent icons.
Gazook89 e7df5d1
remove `experimental`/beta tags on snippets.
Gazook89 f716c39
Add some defaults to dropdown
Gazook89 fa07af2
Add tests for the Dropdown structure and behavior
Gazook89 9c21e6d
update the reset to better vertically align icons
Gazook89 03890fd
Update package-lock.json
Gazook89 cf8f809
Cleanup some loose ends.
Gazook89 a3469a1
Move tests to `tests/components` directory
Gazook89 1861d03
add key to dropdown menuitem
Gazook89 e02ce4f
Merge remote-tracking branch 'upstream/master'
Gazook89 80e1489
Update package-lock.json
Gazook89 6e86b10
Update package-lock.json
Gazook89 1fe1957
change anchor method
Gazook89 b791bce
revert the work done in earlier commit with anchor properties
Gazook89 2164f7a
Merge remote-tracking branch 'upstream/master'
Gazook89 5c50f35
Merge branch 'master' into Better-Dropdowns
Gazook89 b62dc7e
Update package-lock.json
Gazook89 3836150
hide arrows on snippetbar triggers, improve spacing
Gazook89 416161e
fix to previous commit.
Gazook89 9048253
revert some unintended changes from an earlier merge
Gazook89 f67ab5b
update brewRenderer.jsx
Gazook89 f6f4fab
remove tests
Gazook89 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import { createRoot } from 'react-dom/client'; | ||
| import Admin from './admin.jsx'; | ||
| import { bootstrapAnchorPositioningPolyfill } from '../components/anchorPositioningPolyfill.js'; | ||
|
|
||
| const props = window.__INITIAL_PROPS__ || {}; | ||
|
|
||
| createRoot(document.getElementById('reactRoot')).render(<Admin {...props} />); | ||
| bootstrapAnchorPositioningPolyfill(); |
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /* | ||
| This file basically checks support for Anchor Positioning API in the browser, | ||
| and then loads the Oddbird polyfill if support is lacking. | ||
| */ | ||
|
|
||
| let polyfillPromise; | ||
|
|
||
| // look for `anchorName` in the computed styles | ||
| const supportsAnchorPositioning = ()=>'anchorName' in document.documentElement.style; | ||
|
|
||
| // wait for initial render | ||
| const afterInitialRender = ()=>new Promise((resolve)=>{ | ||
| requestAnimationFrame(()=>{ | ||
| requestAnimationFrame(resolve); | ||
| }); | ||
| }); | ||
|
|
||
| export const bootstrapAnchorPositioningPolyfill = ()=>{ | ||
| if(supportsAnchorPositioning()) return Promise.resolve(false); | ||
| if(polyfillPromise) return polyfillPromise; | ||
|
|
||
| polyfillPromise = afterInitialRender() | ||
| .then(async ()=>{ | ||
| const { default: polyfill } = await import('@oddbird/css-anchor-positioning/fn'); | ||
| await polyfill(); | ||
| return true; | ||
| }) | ||
| .catch((error)=>{ | ||
| polyfillPromise = undefined; | ||
| throw error; | ||
| }); | ||
|
|
||
| return polyfillPromise; | ||
| }; | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| import React from 'react'; | ||
| import { Dropdown } from './dropdown.jsx'; | ||
| import { createOpenMenu, renderDropdown, setupDropdownTestLifecycle } from './dropdown.testUtils.js'; | ||
|
|
||
| setupDropdownTestLifecycle(); | ||
|
|
||
| describe('Dropdown behavior', ()=>{ | ||
| it('calls child click handlers when menu items are clicked', async ()=>{ | ||
| const onClick = jest.fn(); | ||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName='Events'> | ||
| <button className='menu-item' role='menuitem' onClick={onClick}>Click Me</button> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| host.querySelector('.menu-list .menu-item').click(); | ||
| expect(onClick).toHaveBeenCalledTimes(1); | ||
|
|
||
| await cleanup(); | ||
| }); | ||
|
|
||
| it('dismisses open menus when clicking a normal menu action', async ()=>{ | ||
| const openMenu1 = createOpenMenu(); | ||
| const openMenu2 = createOpenMenu(); | ||
| const querySpy = jest.spyOn(document, 'querySelectorAll').mockImplementation((selector)=>{ | ||
| if(selector === '.menu-list:popover-open') return [openMenu1, openMenu2]; | ||
| return []; | ||
| }); | ||
|
|
||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName='Dismiss'> | ||
| <button className='menu-item' role='menuitem'>Close</button> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| host.querySelector('.menu-list .menu-item').click(); | ||
|
|
||
| expect(querySpy).toHaveBeenCalledWith('.menu-list:popover-open'); | ||
| expect(openMenu1.hidePopover).toHaveBeenCalledTimes(1); | ||
| expect(openMenu2.hidePopover).toHaveBeenCalledTimes(1); | ||
|
|
||
| await cleanup(); | ||
| }); | ||
|
|
||
| it('does not dismiss when no-dismiss is set to empty string or true', async ()=>{ | ||
| for (const noDismissValue of ['', 'true']) { | ||
| const openMenu = createOpenMenu(); | ||
| const querySpy = jest.spyOn(document, 'querySelectorAll').mockImplementation((selector)=>{ | ||
| if(selector === '.menu-list:popover-open') return [openMenu]; | ||
| return []; | ||
| }); | ||
|
|
||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName={`No Dismiss ${noDismissValue || 'empty'}`}> | ||
| <button className='menu-item' role='menuitem' no-dismiss={noDismissValue}>Stay Open</button> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| host.querySelector('.menu-list .menu-item').click(); | ||
| expect(querySpy).not.toHaveBeenCalled(); | ||
| expect(openMenu.hidePopover).not.toHaveBeenCalled(); | ||
|
|
||
| await cleanup(); | ||
| querySpy.mockRestore(); | ||
| } | ||
| }); | ||
|
|
||
| it('does dismiss when no-dismiss is explicitly false', async ()=>{ | ||
| const openMenu = createOpenMenu(); | ||
| const querySpy = jest.spyOn(document, 'querySelectorAll').mockImplementation((selector)=>{ | ||
| if(selector === '.menu-list:popover-open') return [openMenu]; | ||
| return []; | ||
| }); | ||
|
|
||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName='No Dismiss False'> | ||
| <button className='menu-item' role='menuitem' no-dismiss='false'>Close</button> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| host.querySelector('.menu-list .menu-item').click(); | ||
|
|
||
| expect(querySpy).toHaveBeenCalledWith('.menu-list:popover-open'); | ||
| expect(openMenu.hidePopover).toHaveBeenCalledTimes(1); | ||
|
|
||
| await cleanup(); | ||
| }); | ||
|
|
||
| it('does not dismiss when clicking submenu trigger actions', async ()=>{ | ||
| const querySpy = jest.spyOn(document, 'querySelectorAll'); | ||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName='Top'> | ||
| <Dropdown groupName='Child'> | ||
| <button className='menu-item' role='menuitem'>Leaf</button> | ||
| </Dropdown> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| const submenuTrigger = host.querySelectorAll('.menu-wrapper > .menu-item')[1]; | ||
| submenuTrigger.click(); | ||
|
|
||
| expect(submenuTrigger.hasAttribute('popovertarget')).toBe(true); | ||
| expect(querySpy).not.toHaveBeenCalledWith('.menu-list:popover-open'); | ||
|
|
||
| await cleanup(); | ||
| }); | ||
|
|
||
| it('handles iframe-click by hiding the open popover for this menu', async ()=>{ | ||
| const { host, cleanup } = await renderDropdown( | ||
| <Dropdown groupName='Iframe Dismiss'> | ||
| <button className='menu-item' role='menuitem'>Item</button> | ||
| </Dropdown> | ||
| ); | ||
|
|
||
| const menu = host.querySelector('.menu-list'); | ||
| menu.matches = jest.fn((selector)=>selector === ':popover-open'); | ||
| menu.hidePopover = jest.fn(); | ||
|
|
||
| document.dispatchEvent(new Event('iframe-click')); | ||
|
|
||
| expect(menu.matches).toHaveBeenCalledWith(':popover-open'); | ||
| expect(menu.hidePopover).toHaveBeenCalledTimes(1); | ||
|
|
||
| await cleanup(); | ||
| }); | ||
| }); |
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| /** | ||
| * A dropdown menu component that uses the Anchor Positioning API to position the elements. It supports nested submenus as well. | ||
| * Anchor Positioning is now supported in all major browsers. If support is needed for older browsers (namely, older firefox) | ||
| * it is possible to use absolute positioning and calculations/resize observers to position things well enough, but adds another layer of complexity to the code. | ||
|
Gazook89 marked this conversation as resolved.
Outdated
|
||
| * | ||
| * As-is, the menus will always open down aligned on left to trigger, submenus open to the right initially. | ||
| * If no space, menus will still open down, but aligned to the right of the trigger. Submenus will flip to the other side of the top menu. | ||
| * This could be customized either in more specific CSS, or as a `direction` prop on the component (in future iterations). | ||
| * | ||
| * @param {string} props.groupName - Name of the menu. Appears as the trigger text. | ||
| * @param {string} [props.icon] - Icon to display in the trigger. | ||
| * @param {string} [props.color] - Color class to add to the trigger. | ||
| * @param {string} [props.className] - Additional classes for the menu wrapper. | ||
| * @param {React.ReactNode} [props.customTrigger] - Custom element to use as a trigger. | ||
| * @param {React.ReactNode} [props.children] - Child elements to render in the menu. | ||
| * @returns {React.JSX.Element} | ||
| */ | ||
|
|
||
| import './dropdown.less'; | ||
| import React, { useEffect, useId, useRef } from 'react'; | ||
| import _ from 'lodash'; | ||
|
|
||
| // use react context to keep track of the menu depth (menus in menus) | ||
| const MenuDepthContext = React.createContext(0); | ||
|
|
||
| const Dropdown = ({ groupName, className = null, icon, children, color = null, customTrigger, ...props })=>{ | ||
| const reactId = useId(); | ||
| const safeId = reactId.replace(/[^a-zA-Z0-9_-]/g, ''); | ||
| const menuId = `${_.kebabCase(groupName)}-${safeId}-menu`; | ||
| const anchorName = `--${menuId}`; | ||
| const depth = React.useContext(MenuDepthContext); | ||
|
|
||
| // A menu is a submenu if depth > 0 | ||
| const isSubMenu = depth > 0; | ||
|
|
||
| const triggerRef = useRef(null); | ||
| const menuRef = useRef(null); | ||
|
|
||
| // use setAttribute instead of the React style prop because React strips unknown CSS | ||
| // properties (like anchor-name) from inline styles in browsers that don't support them. | ||
| // setAttribute writes raw CSS text that the anchor positioning polyfill can read | ||
| useEffect(()=>{ | ||
| triggerRef.current?.setAttribute('style', `anchor-name: ${anchorName}`); | ||
| menuRef.current?.setAttribute('style', `position-anchor: ${anchorName}`); | ||
| }, [anchorName]); | ||
|
|
||
| // hide popover with click inside iframe (not supported by light dismiss) | ||
| useEffect(()=>{ | ||
| const menuElement = document.getElementById(menuId); | ||
| if(!menuElement) return; | ||
|
|
||
| const handleClick = ()=>{ | ||
| if(menuElement.matches(':popover-open')) { | ||
| menuElement.hidePopover(); | ||
| } | ||
| }; | ||
| // Listen for clicks from both the main document and the iframe | ||
| document.addEventListener('iframe-click', handleClick); | ||
| return ()=>{ | ||
| document.removeEventListener('iframe-click', handleClick); | ||
| }; | ||
| }, [menuId]); | ||
|
|
||
| // the trigger is the piece placed inside the opening button of the menu. | ||
| // This method allows for creating a generic span with the group name, | ||
| // or using a bespoke element (like a graphic) passed in from props to be used as the trigger | ||
| const trigger = (groupName = 'menu', icon = '')=>{ | ||
| if(!customTrigger){ | ||
| return <> | ||
| <i className={icon}></i><span className='menu-name'>{groupName}</span><i className={`caret fas fa-caret-${isSubMenu ? 'right' : 'down'}`}></i> | ||
| </>; | ||
| } else { | ||
| return customTrigger; | ||
| } | ||
| }; | ||
|
|
||
| // handle clicks on menu items. By default, actions do dismiss. | ||
| const handleMenuActionClick = (event)=>{ | ||
| const menuElement = menuRef.current; | ||
| if(!menuElement) return; | ||
|
|
||
| const menuAction = event.target.closest('button, a, [role="menuitem"]'); | ||
| if(!menuAction || !menuElement.contains(menuAction)) return; | ||
|
|
||
| // don't dismiss if the target triggers a submenu | ||
| if(menuAction.hasAttribute('popovertarget')) return; | ||
|
|
||
| // don't dismiss if the target has `no-dismiss` attribute | ||
| const noDismissValue = menuAction.getAttribute('no-dismiss')?.toLowerCase(); | ||
| if(noDismissValue === '' || noDismissValue === 'true') return; | ||
|
|
||
| document.querySelectorAll('.menu-list:popover-open').forEach((openMenu)=>openMenu.hidePopover()); | ||
| }; | ||
|
|
||
| return ( | ||
| <div className={['menu-wrapper', className].join(' ')} role='none' > | ||
| <button | ||
| id={`${menuId}-trigger`} | ||
| className={['menu-item', color].join(' ')} | ||
| popoverTarget={menuId} | ||
| aria-haspopup='menu' | ||
| role='menuitem' | ||
| disabled={!React.Children.count(children)} | ||
| ref={triggerRef} | ||
| > | ||
| {trigger(groupName, icon)} | ||
| </button> | ||
| <MenuDepthContext.Provider value={depth + 1}> | ||
| <div | ||
| ref={menuRef} | ||
| id={menuId} | ||
| className='menu-list' | ||
| popover='auto' | ||
| role='menu' | ||
| onClick={handleMenuActionClick} | ||
| > | ||
| {children} | ||
| </div> | ||
| </MenuDepthContext.Provider> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export { Dropdown }; | ||
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| .menu-wrapper { | ||
| position: relative; | ||
| place-content: center; | ||
|
Gazook89 marked this conversation as resolved.
Outdated
|
||
| &:is(.menu-bar > .menu-section > .menu-wrapper){ | ||
| display: inline-block; | ||
| } | ||
| } | ||
|
|
||
| .menu-list { | ||
| position : fixed; | ||
| z-index : 1000; | ||
| inset-block-start: anchor(bottom); | ||
| inset-inline-start: anchor(left); | ||
| position-try: flip-inline; | ||
| color: unset; // [popover] gets a `canvastext` color value from useragent. | ||
|
Gazook89 marked this conversation as resolved.
Outdated
|
||
| > .menu-wrapper { | ||
| position:relative; | ||
| > .menu-list { | ||
| margin: 0 0px; | ||
| inset-block-start: anchor(top); | ||
| inset-inline-start: anchor(right); | ||
| position-try: flip-inline; | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.