Fluentui: Menu of CommandButton not dismissed as documented
Environment Information
- Package version(s): 7.115.4
Please provide a reproduction of the bug in a codepen:
Actual behavior:
Menu of CommandButtonstays open after clicking item which has onClick() handler defined which returns true.
Expected behavior:
Menu should close according to documentation of onClick():
https://developer.microsoft.com/en-us/fluentui#/controls/web/contextualmenu#IContextualMenuItem
Callback for when the menu item is invoked. If ev.preventDefault() is called in onClick, the click will not close the menu. Returning true will dismiss the menu even if ev.preventDefault() was called.
LukeOwlclaw
All 8 comments
Workaround:
Set
ev.defaultPrevented = false;
right after calling
ev.preventDefault();
@LukeOwlclaw Thanks for reaching out to us! I went to your codepen but it doesn't render anything... I see only blank white space. It also looks like you already have a workaround which is great! I wonder if we need to change the documentation to reflect this though? @ecraig12345
aneeshack4
on 27 May 2020
I debugged it and this is the issue (last part):
https://github.com/microsoft/fluentui/blob/cc95baee6960fd47f1eca680d4f9b5d9efaf37d4/packages/office-ui-fabric-react/src/components/Button/BaseButton.tsx#L533-L542
I agree with Aneesha that we should probably just remove Returning true will dismiss the menu even if ev.preventDefault() was called. from the documentation--that behavior seems a bit weird. @joschect does that sound okay to you?
ecraig12345
on 30 May 2020
@joschect is this something you're looking at or is this something that @khmakoto should look at?
paulgildea
on 2 Jun 2020
My 2 cents is that we should also just remove that statement from the docs.
khmakoto
on 2 Jun 2020
@ecraig12345 We can't remove it from the documentation entirely. It's based in the contextualmenu, where it does work as expected in that the onDismiss function is called if you return true. It is a little weird but it provides a useful way to force the menu to close, even if something handled the event. Realistically it should return true/false and that's the only basis that dismissal should work on.
I don't know the initial intent of button but it is weird that prevent default checked in several places. We could, in the short term, just clear up the Button docks but we need to ensure that the contextualmenu docs remain unchanged since that behavior is expected. https://github.com/microsoft/fluentui/blob/master/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx#L1199-L1206 Relevant lines from the contextualmenu
joschect
on 2 Jun 2020
They both use the same docs in this case (since the property is from the same interface), so we unfortunately would have to just add a statement explaining the different behavior of different components.
ecraig12345
on 3 Jun 2020
:tada:This issue was addressed in #13471, which has now been successfully released as [email protected].:tada:
Handy links:
msft-github-bot
on 8 Jun 2020
Related issues
VincentBailly
路
3Comments
satbai
路
3Comments
carruthe
路
3Comments
mattcoxonline
路
3Comments
rickyp-ms
路
3Comments
Most helpful comment
@ecraig12345 We can't remove it from the documentation entirely. It's based in the contextualmenu, where it does work as expected in that the onDismiss function is called if you return true. It is a little weird but it provides a useful way to force the menu to close, even if something handled the event. Realistically it should return true/false and that's the only basis that dismissal should work on.
I don't know the initial intent of button but it is weird that prevent default checked in several places. We could, in the short term, just clear up the Button docks but we need to ensure that the contextualmenu docs remain unchanged since that behavior is expected. https://github.com/microsoft/fluentui/blob/master/packages/office-ui-fabric-react/src/components/ContextualMenu/ContextualMenu.base.tsx#L1199-L1206 Relevant lines from the contextualmenu