grommet / hpe-design-system

HPE Design System
48 stars 23 forks source link

MENU - Refactor menu component in Figma #3898

Closed julauxen closed 2 months ago

julauxen commented 3 months ago

Who is requesting this fix? (add contact details here) Seamus

Where is this fix needed? (links and screenshots) Current menu component in figma library

Expected Behaviour (tell us what should happen)

  1. When menu is 'open' there should be no content shift in auto-layout groups that contain (eg page header).
  2. Product team should be alse to choose number of menu items (up to a reasonable maximum)
  3. The menu is triggerd by a button, but currently thats not the case in the component - It should be consuming the button atom.

Current Behaviour (show us the current behaviour)

  1. When default menu component is set to open, the open menu options takes up vertical space - this doesnt mimich how the component behaves in code in the browser
  2. Currently the 'open menu dropdown' height is controlled by manually adjusting the container and revealing form clipped/hidden menu items - this means there are almost always 'hidden' items on the artboard - could lead to confusion in dev handoff
  3. The menu item had a 'spacer' on bottom - in grommet its top.

    Reviewers:

List possible consequences, involved assets and components:

  1. Menu will need refactor & likely deprecation of cold component (tbd)
  2. Menu is consumed by pageHeader, Toolbar - Need audit to complete list of impacted components
  3. Need to test if product teams designers are comfortable with "select number of menu items" concept

Checklist (add your progress in the comments)

SeamusLeonardHPE commented 3 months ago

Aim to unblock #3290 with this enhancement

@CillianHPE @KennyAtHPE can you guys review my proposed refactor of menu, happy to connect to do a demo if it suits.

Main changes are -

  1. It consumes button
  2. Open dropdown behaviour mimics web (absolute positioned)
  3. Product designer chooses number of menu items

Working file for review: https://www.figma.com/design/RY0J9JZwWicKELr6j84GVh/%233898-Menu-Refactor?node-id=0-1&t=Gy50ZoxHmMmNDiNn-1

SeamusLeonardHPE commented 3 months ago

@ashifalinadaf @CillianHPE @KennyAtHPE

Did a refactor using the exiting components so as to not deprecate the published work.

Changes .Menu Item

.Menu List

Menu

https://www.figma.com/design/5uOrsL2qk8XwwH8C1ZpDb6/branch/Ih7KFTx8RdOr13yQIzqvsW/HPE-Design-System-Components?m=auto&node-id=6062-109930&t=PwkmQbc9BYpm0JIK-1

Question raised by Cillian - should we add icon variant/boolean to .Menu Item as its available in grommet https://storybook.grommet.io/?path=/story/controls-menu-item-with-icon--item-with-icon

vavalos5 commented 3 months ago

@julauxen any updates here on progress?

Update: I just seen Seamus's comment on PageHeader on how PH might be unblocked depending on outcome of this.

SeamusLeonardHPE commented 3 months ago

@vavalos5 I was assigned this on an waiting for review from @KennyAtHPE If you have time I'll add you on figma as a reviewer of the branch.

One question that still need addressed, I added a min & max width to the open menu, but grommet seems to allow unlimited width on the menu items (guidance dictates that most are short single word action verbs) So should I remove the min/max, or dos it help enforce guidance?

Oh also I have a question for D&D today asking if we should include an icon variant in the menu items.

vavalos5 commented 3 months ago

Thanks for the update @SeamusLeonardHPE. I see I accidentally tagged Julia -oops! 😁

I'll review your Menu Figma file and provide feedback there. Ty!!

vavalos5 commented 3 months ago

Left feedback on your Figma file. :) Looks good! I have some small questions on there.

KennyAtHPE commented 3 months ago

Left comments on the file. Looks great so far!

SeamusLeonardHPE commented 2 months ago

Addressed review feedback in the branch & merged into main. Will be published today pending a question from the Ds slick channel.

SeamusLeonardHPE commented 2 months ago

Pulished & closed with note:

Figma component library

Refactored component

Impacted / Modified components

Component details


Menu

Impact



Toolbar

Impact