Skip to content
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

Flatten Menu data #796

Merged
merged 3 commits into from Nov 19, 2018
Merged

Flatten Menu data #796

merged 3 commits into from Nov 19, 2018

Conversation

brentertz
Copy link
Member

@brentertz brentertz commented Nov 16, 2018

Description

Flatten data structure for Menu groups

  • Groups are now created similarly to dividers, using { group: true, title: 'Group Title' }
  • Renders all menu items, including groups, as flat DOM structure
  • The children API for MenuGroups has not changed

Motivation and context

Connects #679

Was investigating performance improvements for Dropdown, Select, using virtualized rendering...

  • Rendering menu items in a flat DOM structure is necessary to support virtualized rendering.
  • Simpler API for end users, though it is a breaking change for anyone using grouped data
  • Simplify Menu rendering code

Screenshots, videos, or demo, if appropriate

https://menu-flat-data--mineral-ui.netlify.com/

How to test

  1. No changes in functionality
  2. Keyboard navigation in Select/Dropdown works as expected
  3. Verify documentation/example updates

Types of changes

  • Refactor
  • Breaking change (fix or feature that would cause existing functionality to change)

The structure of the data for Menu, Dropdown, Select, and CardTitle components has changed. Previously, grouped data was provided in a nested data structure. A flat array of items is now required. Groups are now created similarly to dividers, using { group: true, title: 'Group Title' }.

The theme export for menuGroupTitleTheme has been renamed to menuGroupTheme. The MenuGroup_margin theme key has been replaced with MenuGroupTitle_marginTop.

Checklist

@brentertz brentertz self-assigned this Nov 16, 2018
@brentertz brentertz requested a review from kylegach Nov 16, 2018
src/library/Menu/themes.js Outdated Show resolved Hide resolved
src/library/Menu/Menu.js Outdated Show resolved Hide resolved
kylegach
kylegach previously requested changes Nov 16, 2018
Copy link
Contributor

@kylegach kylegach left a comment

I like this change, but I think the styles and maybe the group data object shape need more consideration (see comments).

@brentertz brentertz force-pushed the menu-flat-data branch 2 times, most recently from 7783719 to fa3a5bb Compare Nov 19, 2018
@brentertz brentertz dismissed kylegach’s stale review Nov 19, 2018

All feedback has been addressed

Brent Ertz added 3 commits Nov 19, 2018
…Menu groups

* Groups are now created similarly to dividers, using { group: true, text: 'Group Title' }
* Renders all menu items, including groups, as flat DOM structure, which is necessary to support virtualized rendering.

BREAKING CHANGE:

The structure of the data for Menu, Dropdown, Select, and CardTitle components has changed.  Previously, grouped data was provided in a nested data structure.  A flat array of items is now required.  Groups are now created similarly to dividers, using { group: true, text: 'Group Title' }. The theme export for `menuGroupTitleTheme` has been renamed to `menuGroupTheme`.  The `MenuGroup_margin` theme key has been removed, and the `MenuGroupTitle_paddingTop` adjusted to compensate accordingly.
@brentertz brentertz merged commit 20db522 into master Nov 19, 2018
3 checks passed
@brentertz brentertz deleted the menu-flat-data branch Nov 19, 2018
@brentertz brentertz removed the review label Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants