fix(Menu): add overflow and max-height so that a scrollbar is visible for long lists #41
Conversation
this will give Menu a scrollbar which is ideal for long lists, it uses the same max-height as the Select component
🦋 Changeset detectedLatest commit: 3a83c98 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
There was a problem hiding this comment.
Thanks for the contribution @kaydwithers. I'm thinking it'd also be great to capture an additional example in the stickersheet stories to showcase the max number of items before the overflow kicks in when the menu is open.
Thanks @mcwinter07, I've added a new "Short list" story. |
| @@ -79,6 +79,29 @@ const StickerSheetTemplate: StoryFn<{ isReversed: boolean }> = ({ | |||
| }) => ( | |||
| <StoryWrapper isReversed={isReversed}> | |||
| <StoryWrapper.RowHeader headings={["Default", "Icon Button"]} /> | |||
|
|
|||
| <StoryWrapper.Row rowTitle="Short list"> | |||
There was a problem hiding this comment.
Thanks for the update @kaydwithers - I have a few small tweaks before I give the PR the green light. Instead of the Short list example being a new row, could we instead move this into the "open" and "closed" with the respective states rows so we can visually capture the these for chromatic. We'll need a title added to headings in ln81.
With an open an close state for the shorter list our snapshots can capture what the menu looks like without the scroll bar so any change to the menu items size impact overflow can be caught in visual regression.
Shout out if you have any further queries :)
There was a problem hiding this comment.
@mcwinter07 I've added short/long list open examples, is that better?
There was a problem hiding this comment.
Main reason I'd say we should have them both in the stickersheet stories is that this is what we send to chromatic. By default our stickersheet stories are the only ones we send to chromatic to reduce the overall spend on snapshots.
Given you've done the legwork by making the individual stories I'm happy to merge this in to get your fix moving and we can use those in the updated docs when we migrate Menu KAIO (@kaizen/components) 👍
Why
This PR adds a scrollbar, similar to the Select component scrollbar.
When a Menu has a lot of list items then the list can sometimes go outside the page. This change will allow a scrollbar to be visible for long lists.
22rem is the same max-height as the Select component.
What
Before
After
Select with scrollbar dropdown for reference.
