This repository has been archived by the owner on Jan 20, 2022. It is now read-only.
AP-1335 Implement Menus #153
Merged
Conversation
This file contains 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
justinanastos
force-pushed
the
justin/AP-1335/menus
branch
3 times, most recently
from
2bed45d to
e21ebdd
Compare
justinanastos
force-pushed
the
justin/AP-1335/menus
branch
4 times, most recently
from
35417c9 to
99dde4e
Compare
justinanastos
force-pushed
the
justin/AP-1335/menus
branch
from
99dde4e to
b34acfd
Compare
cheapsteak
approved these changes
Feb 3, 2020
There was a problem hiding this comment.
Looks great!
I think this should integrate pretty well with downshift? Keyboard navigation would be great for a11y and convenience (especially for search), but makes sense to do that in the consuming package (engine-frontend) rather than the component lib
|
Great observation @cheapsteak ; it'll totally work with downshift! |
|
🚀 PR was released in v2.25.0 🚀 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Contributes to https://apollographql.atlassian.net/browse/AP-1335
Intent
Create a menu system based on https://zpl.io/adX0ELe. Check the examples in the storybook docs to see how to compose menus together!
Uses tippy.js under the hood, just like our tooltips.
This has a bug that can't be fixed until atomiks/tippyjs#678 lands sometime in February. Popper v1 can't reliably figure out the boundaries of a tooltip nested below a component using overflow scrolling. This is resolved in popper v2; but that's not yet included in tippy because it's a breaking change. Fortunately the project maintainers are the same people so they're working on it.
It was a challenge to make the content scrollable if it overflowed the container, but that's included.
Tests
There aren't any. If you can think of any worth adding, I'd love to!
Multilevel Menus
These are supported out of the box with the underlying tippy.js library; we just haven't implemented it here yet, which is fine.
Details
This includes these new components:
MenuThe parent of a menu. Nesting doesn't work yet; we'll follow up with that if/when we need it. Tippy does support it, we just have to set it up.
MenuItemItem in the menu
MenuHeaderHeading for a menu. Composed from
MenuItemto inherit it's stylesMenuDividerFor divided menus
MenuIconSizeThis is not exported but used internally so you can set the menu size on a
Menuand then use context to access the desired menu size for all children.Release Notes
Create new
Menucomponent and supporting componentsMenuItem,MenuHeader, andMenuDivider.See https://zpl.io/adX0ELe for the original designs and the storybook docs page for usage instructions and examples.
This does not include any tests because I didn't know what to test. Any tests or ideas for tests would be most welcome!