refactor(ui5-menu): remove references to parent #11217
Conversation
| @@ -46,7 +46,7 @@ function rightContent(this: MenuItem) { | |||
| </div> | |||
| ); | |||
| case this.hasEndContent: | |||
| return <slot name="endContent"></slot>; | |||
| return <slot name="endContent" onKeyDown={this._endContentKeyDown}></slot>; | |||
There was a problem hiding this comment.
Im not sure if handling events is a good practice on slot level, maybe wrap it in <div> and handle the event from there.
@tsanislavgatev could you give your opinion on this, as I am not entirely sure.
There was a problem hiding this comment.
The solution was discussed with Tsani, but he could have had something else in mind :)
Putting it in a div causes styling issues and potentially other issues I can't think of right now.
packages/main/src/Menu.ts
Outdated
| this._closeItemSubMenu(openedSibling); | ||
| const menuItems = this._menuItems; | ||
| if (menuItems.indexOf(item) > -1) { | ||
| menuItems.forEach(menuItem => { menuItem !== item && menuItem._close(); }); |
There was a problem hiding this comment.
This line is hard to read. We have a convention to keep the && operator for boolean checks only and not chain it with method calls.
Even though it would be a little bit longer code wise, please use if.
There was a problem hiding this comment.
Took out the logic in a function instead, but the naming could probably be improved (_closeOtherSubMenus)
I'm hesitant on calling it _closeSiblingSubMenus
Refactored submenu closing and navigation handling, removing references to the parent element:
exit-end-contentevent, which is handled by the parent Menu or MenuItem.