feat(menu): Icons

[miroslavstastny]

API Proposal

stardust-ui/react-old#121

Menu Items with icons

MenuItems can now contain icons:

const items = [
  { key: 'home', content: 'Home', icon: { name: 'home', size: 'big' } },
  { key: 'users', content: 'Users', icon: 'users' },
  { key: 'search', icon: 'search' },
]

class MenuExampleWithIconsShorthand extends React.Component {
  render() {
    return <Menu defaultActiveIndex={0} items={items} />
  }
}

Menu can also consist of Icons only (no text):

const items = [
  { key: 'home', icon: {name: 'home', size: 'big'} },
  { key: 'users', icon: 'users' },
  { key: 'search', icon: 'search' },
]

class MenuExampleIconsOnlyShorthand extends React.Component {
  render() {
    return <Menu icons defaultActiveIndex={0} items={items} />
  }
}

Icons only menu intentionally has no spacing between items. In Teams, the free space around the icons is part of the SVG icons. If there is spacing required, it can be achived using IMenuVariables.iconsMenuItemSpacing

Problems

• At the moment, it is not possible to create Menu with icons using Children API. We agreed that (for now) only Shorthand API will be supported for Menu.
• Icons only menu hover and active states do not match spec as there is no support for hover icons in stardust yet.

TODO

• Conformance test
• Minimal doc site example
• Stardust base theme
• Teams Light theme
• Teams Dark theme
• Teams Contrast theme
• Confirm RTL usage
• W3 accessibility check
• Stardust accessibility check
• Update glossary props table
• Update the CHANGELOG.md

[codecov]

Codecov Report

Merging #73 into master will decrease coverage by 0.21%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master      #73      +/-   ##
==========================================
- Coverage   86.27%   86.05%   -0.22%     
==========================================
  Files          39       39              
  Lines         663      667       +4     
  Branches       99      102       +3     
==========================================
+ Hits          572      574       +2     
- Misses         88       90       +2     
  Partials        3        3

Impacted Files	Coverage Δ
src/components/Menu/Menu.tsx	100% <100%> (ø)	⬆️
src/components/Menu/MenuItem.tsx	92.3% <60%> (-7.7%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb527f4...bf34c57. Read the comment docs.

[mnajdova]

Should we consider using the Layout component for aligning the content inside the MenuItem? Are there going to be other types of menus with media in the start and end?

[miroslavstastny]

Good point. I would suggest leaving it as it is for now and refactor by adding Layout later as new requirements for the component come.

[mnajdova]

Sure!

[levithomason]

This could be stateless for brevity.

[levithomason]

If we are removing shorthand, shouldn't this example be removed?

[levithomason]

Titles should reflect props when possible:
"Vertical Icon" for <Menu vertical icon />

[levithomason]

Please use destructuring for consistency.

[levithomason]

icon and icons props may confuse users. Proposal:

There is no icon prop for Menu yet, propose we use <Menu icon /> to signify the type of the menu.

There is an icon prop already for Menu.Item, propose we use <Menu.item iconOnly /> to signify there is an icon only and no content.

Thoughts?

[miroslavstastny]

• Agree that icon vs icons on MenuItem is confusing.
• But in your proposal icon property means something different in Menu and MenuItem - are we ok with that?
• Or we can use iconOnly in Menu as well.

[levithomason]

Let's go with iconOnly for now, I will post an RFC about icon type components. There are many considerations beyond the scope of this PR.