feat(menu): Submenu #539

jurokapsiar · 2018-11-29T07:37:25Z

Based on two previous menu/submenu PRs. Fixes #542

Missing things:

document click / focus out should close submenu
action should close submenu
left/right key should move to the next/previous menu item according to ARIA practices
submenu indicator
commented out overflow, not sure why it was there, need to verify
verify wrapper render

Update by @mnajdova
Some questions:

I needed to add key to the wrapper element created, so in the defaultProps I added key that is generated by a new added dependency uniqid. Are we ok with this?
The tests are failing, I guess because I added Ref wrapper around the ElementType. How can be tackle this? Is the wrapper maybe causing this?
How are we handling event handlers when we have wrapper? There is issue with the onClick event, which is added on the wrapper, but on the anchor is added only if the wrapper is not added. Am I doing something wrong here?

The questions were resolved thanks to @layershifter

All accessibility requirements are added, let me know if you find some issue with it.

layershifter · 2018-11-29T10:02:32Z

Should we use the kind prop to simplify the render logic, #512?

jurokapsiar · 2018-11-29T10:41:51Z

src/themes/teams/components/Menu/menuStyles.ts

@@ -28,7 +28,7 @@ export default {
            ...solidBorder(variables.primaryBorderColor),
          }),
          borderRadius: pxToRem(4),
-          overflow: 'hidden',
+          // overflow: 'hidden',


this needs to be verified - it breaks submenu when oveflow is hidden

This is the PR that added it: #360 Will sync with @Bugaa92 for finding alternative solution for the cropped borders, I have new ideas.

src/themes/teams/components/Menu/menuVariables.ts

-small fixes in the styles and the way the submenu is generated

…be done for enter/space) -applied consistent (left-right) navigation for horizontal menu and (up-down) navigation for vertical menu

# Conflicts: # src/components/Menu/Menu.tsx # src/components/Menu/MenuItem.tsx

-implemented enter key on leaf menu item to close the menu

src/components/Menu/MenuItem.tsx

src/components/Menu/Menu.tsx

-fixed issue with the condition for the active prop

-added correct typings to the menuStyles

mnajdova · 2018-12-18T18:17:41Z

All comments are resolved. I changed the look of the hovered/focused vs active styles. Couple of examples of the variants:

…ild menu items and the last child menu items in vertical menu

kuzhelov · 2018-12-19T13:20:59Z

docs/src/examples/components/Menu/Variations/MenuExampleIconOnlyPrimaryInverted.shorthand.tsx

-      defaultColor: siteVars.gray06,
-      defaultBackgroundColor: siteVars.brand,
-      typePrimaryActiveBorderColor: siteVars.white,
+      color: siteVars.gray06,


src/components/Menu/MenuItem.tsx

kuzhelov · 2018-12-19T13:27:02Z

src/components/Menu/MenuItem.tsx

+    const { active, menu } = this.props
+    if (menu) {
+      if (doesNodeContainClick(this.menuRef.current, e)) {
+        // submenu was clicked => close it and propagate


thanks for comments, very helpful ones!

src/components/Menu/MenuItem.tsx

* implement caching strategy * adjust file name of scan marker * add yarn lock hash to marker file name * add change to build config * fix dir name in build config * improve caching strategy * just restore cache * temporary remove lint and tests * try * fix caching strategy * try * try * try * try epoch * create file on scan * return lint and test steps * introduce comment for the caching approach taken * remove unnecessary function * simplify expression for marker file name

* feat(text): color prop * addressed comments * changelog * amended changelog * made text color override other props that change color

* feat(header): header and header description color prop * changelog * fixed examples * addressed PR comments

* introduce offset prop * correct description of supported values * update changelog * introduce fix * ensure RTL is properly applied to complex offset expressions * rename method to make logic more expressive * add unit tests * remove unnecessary grid props from offset example * update changelog

* Reflect which item is selected in list * Make list derived from autocontrolled component * small fix * Update ListExampleSelection.tsx * Update ListExampleSelection.shorthand.tsx * Small improvement * Rename *ItemIndex -> *Index * Names refactoring * Minor improvements * update changelog * Add onSelectedIndexChange * Add some tests * Small improvements afer CR * Small improvements afer CR * Small improvements afer CR * create focus handler when List is constructed * fix changelog * changelog

* docs(Examples): allow to use TS in examples * add jsdoc * fix typo * rename file * add comment * `createExample` to `createExampleSourceCode` * rework with `path.relative()` * remove JSON files on remove tsx * create getRelativePathToSource function

# Conflicts: # CHANGELOG.md

-removed TODO comment

# Conflicts: # CHANGELOG.md

Gopal Goel and others added 10 commits October 12, 2018 19:02

feat(menuItem: add menu prop)

72ea550

Only one submenu open at a time

d4a3a0c

Fix bug: submenu noe closes on clicking a menuItem with no submenu

85981f8

Code cleanup

fa99935

Remove the activeIndex prop passed to the menuItem

8271a18

Remove Popup and implement submenu without it

35e8a76

Merge branch 'master' into feat/menuitem-menu-prop

756f7fa

initial keyboard support

bea260a

left/right arrow handling

8f678ec

simplify keyboard handlers

0f4be8b

jurokapsiar requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners November 29, 2018 07:37

jurokapsiar commented Nov 29, 2018

View reviewed changes

src/themes/teams/components/Menu/menuVariables.ts Outdated Show resolved Hide resolved

jurokapsiar mentioned this pull request Nov 29, 2018

[RFC] Submenu functionality in Menu component #542

Closed

alinais added this to mnajdova in Core Team Nov 29, 2018

jurokapsiar added the 🚧 WIP label Nov 30, 2018

-fixed import in the MenuItem

46d52c6

mnajdova self-assigned this Nov 30, 2018

manajdov added 3 commits November 30, 2018 15:29

-added submenuIndicator

c1ad2da

-small fixes in the styles and the way the submenu is generated

-clicking on leaf element should close the submenus (the same should …

acee5ee

…be done for enter/space) -applied consistent (left-right) navigation for horizontal menu and (up-down) navigation for vertical menu

Merge branch 'master' into feat/menuitem-menu-prop

4b4976f

# Conflicts: # src/components/Menu/Menu.tsx # src/components/Menu/MenuItem.tsx

jurokapsiar requested a review from layershifter as a code owner December 7, 2018 11:39

manajdov added 2 commits December 7, 2018 15:13

-implemented outside click to close all menus

5575653

-implemented enter key on leaf menu item to close the menu

-fixed import

7a17afb

sophieH29 reviewed Dec 7, 2018

View reviewed changes

src/components/Menu/MenuItem.tsx Outdated Show resolved Hide resolved

layershifter reviewed Dec 18, 2018

View reviewed changes

src/components/Menu/Menu.tsx Outdated Show resolved Hide resolved

manajdov added 3 commits December 18, 2018 18:46

-improved example

2ece67b

-fixed issue with the condition for the active prop

-exported MenuState

a97a2fe

-added correct typings to the menuStyles

-fixed underlined active + hovered style

c590f3e

-fixed border corner clipped by adding custom styles for the first ch…

77342ea

…ild menu items and the last child menu items in vertical menu

kuzhelov reviewed Dec 19, 2018

View reviewed changes

kuzhelov added 🚀 ready for review and removed needs author feedback Author's opinion is asked labels Dec 19, 2018

-addressed comments on PR

69fcf38

kuzhelov approved these changes Dec 19, 2018

View reviewed changes

miroslavstastny and others added 16 commits December 19, 2018 16:19

chore: prepare release 0.15.0 [ci skip]

54f0a69

0.15.0

9909483

fix(Prototype): Fix Popover prototype after breaking changes (#623)

fa2e9bd

feat(text): color prop (#597)

1d8e168

* feat(text): color prop * addressed comments * changelog * amended changelog * made text color override other props that change color

feat(header): header and header description color prop (#628)

fa78e86

* feat(header): header and header description color prop * changelog * fixed examples * addressed PR comments

-updated changelog

e5aa7b3

Merge branch 'master' into feat/menuitem-menu-prop

14eb199

# Conflicts: # CHANGELOG.md

-updated changelog

3c55948

-updated changelog with breaking changes

6aac80b

-removed TODO comment

Merge branch 'master' into feat/menuitem-menu-prop

d2d3412

# Conflicts: # CHANGELOG.md

Merge branch 'master' into feat/menuitem-menu-prop

1849f5f

-fixed imports in examples

e592039

mnajdova merged commit 5324ba9 into master Dec 19, 2018

mnajdova mentioned this pull request Dec 19, 2018

The subscription for EventStack should be refactored in the Popup and MenuItem components #649

Closed

layershifter deleted the feat/menuitem-menu-prop branch January 10, 2019 11:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(menu): Submenu #539

feat(menu): Submenu #539

jurokapsiar commented Nov 29, 2018 •

edited by mnajdova

layershifter commented Nov 29, 2018

jurokapsiar Nov 29, 2018

mnajdova Dec 16, 2018

mnajdova commented Dec 18, 2018

kuzhelov Dec 19, 2018

kuzhelov Dec 19, 2018

feat(menu): Submenu #539

feat(menu): Submenu #539

Conversation

jurokapsiar commented Nov 29, 2018 • edited by mnajdova

layershifter commented Nov 29, 2018

jurokapsiar Nov 29, 2018

Choose a reason for hiding this comment

mnajdova Dec 16, 2018

Choose a reason for hiding this comment

mnajdova commented Dec 18, 2018

kuzhelov Dec 19, 2018

Choose a reason for hiding this comment

kuzhelov Dec 19, 2018

Choose a reason for hiding this comment

jurokapsiar commented Nov 29, 2018 •

edited by mnajdova