Add possibility to render submenu in MenuItem#126
Conversation
sophieH29
requested review from
dvdzkwsk,
kuzhelov,
levithomason,
miroslavstastny and
mnajdova
as code owners
Having a concern regarding style:Menu and menu items are having a transparent background. So when a submenu is rendered, it can look weird, as everything behind it is visible. Looking forward to your thoughts and ideas! |
Codecov Report
@@ Coverage Diff @@
## master #126 +/- ##
==========================================
+ Coverage 67.82% 67.97% +0.15%
==========================================
Files 101 101
Lines 1386 1396 +10
Branches 275 269 -6
==========================================
+ Hits 940 949 +9
- Misses 443 444 +1
Partials 3 3
Continue to review full report at Codecov.
|
|
|
||
| /** Initial submenuOpened value. */ | ||
| defaultSubmenuOpened: PropTypes.bool, | ||
|
|
There was a problem hiding this comment.
This comment will be used in the docs page for describing the property. Please use more descriptive comment avoiding any properties.
There was a problem hiding this comment.
@mnajdova that makes sense. But I've made the same description as for defaultActiveindex prop in Accordion and Menu components to keep consistency. Should I change it?
There was a problem hiding this comment.
Aah, alright then, leave it as is.
|
I'd expect the submenu to have the same color as the menu bar. Is it possible to apply this style? |
|
|
||
| const MenuExampleWithSubmenu = () => ( | ||
| <Menu defaultActiveIndex={0} items={items} variables={menuWidthVariable} /> | ||
| ) |
There was a problem hiding this comment.
Please try to add some height on the element as the content is not visible (try to wrap the Menu in some wrapper with some bigger height).
There was a problem hiding this comment.
Also, I would expect to be able to expand only one submenu at a time. I don't think the scenario above (having three different dropdowns should work (try closing the submenu if there is click outside of it).
| position: 'absolute', | ||
| borderTopLeftRadius: 0, | ||
| borderTopRightRadius: 0, | ||
| }, |
There was a problem hiding this comment.
Totally support @notandrew comment, the type should affect the submenu as well.
|
@priyankar205 and @gopalgoel19 please review this and collaborate. It seems there is some overlapping work here. |
|
Closing as it doesn't support our immediate needs right now |
5 participants
MenuItem
This PR introduces possibility to add vertical submenu rendered in MenuItem.
TODO
API Proposal
Submenu