This repository was archived by the owner on Mar 4, 2020. It is now read-only.
feat(menu): Submenu #539
Merged
feat(menu): Submenu #539
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should we use the |
jurokapsiar
commented
Nov 29, 2018
| @@ -28,7 +28,7 @@ export default { | |||
| ...solidBorder(variables.primaryBorderColor), | |||
| }), | |||
| borderRadius: pxToRem(4), | |||
| overflow: 'hidden', | |||
| // overflow: 'hidden', | |||
There was a problem hiding this comment.
this needs to be verified - it breaks submenu when oveflow is hidden
There was a problem hiding this comment.
This is the PR that added it: #360 Will sync with @Bugaa92 for finding alternative solution for the cropped borders, I have new ideas.
jurokapsiar
commented
Nov 29, 2018
added 3 commits
November 30, 2018 15:29
-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
added 2 commits
December 7, 2018 15:13
-implemented enter key on leaf menu item to close the menu
sophieH29
reviewed
Dec 7, 2018
added 3 commits
December 18, 2018 18:46
-fixed issue with the condition for the active prop
-added correct typings to the menuStyles
|
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
reviewed
Dec 19, 2018
| defaultColor: siteVars.gray06, | ||
| defaultBackgroundColor: siteVars.brand, | ||
| typePrimaryActiveBorderColor: siteVars.white, | ||
| color: siteVars.gray06, |
| const { active, menu } = this.props | ||
| if (menu) { | ||
| if (doesNodeContainClick(this.menuRef.current, e)) { | ||
| // submenu was clicked => close it and propagate |
There was a problem hiding this comment.
thanks for comments, very helpful ones!
kuzhelov
approved these changes
Dec 19, 2018
* 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on two previous menu/submenu PRs. Fixes #542
Missing things:
Update by @mnajdova
Some questions:
uniqid. Are we ok with this?Refwrapper around theElementType. How can be tackle this? Is the wrapper maybe causing this?The questions were resolved thanks to @layershifter
All accessibility requirements are added, let me know if you find some issue with it.