-
Notifications
You must be signed in to change notification settings - Fork 705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add check to only open flyout if a menubaritem has items #2889
Add check to only open flyout if a menubaritem has items #2889
Conversation
@@ -55,6 +55,8 @@ | |||
<MenuFlyoutItem Text="Word Wrap"/> | |||
<MenuFlyoutItem Text="Font..."/> | |||
</muxc:MenuBarItem> | |||
|
|||
<muxc:MenuBarItem Title="No children"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chingucoding. Could we add a test ? I think you can click and look at all the open popups to validate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added new interaction test now.
[TestMethod] | ||
public void EmptyMenuBarItemNoPopupTest() | ||
{ | ||
if (PlatformConfiguration.IsDevice(DeviceType.Phone)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need that? I don't think we are doing any tests on Windows Mobile devices any longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought the same. I think we should look at all test holistically at some point and remove obsolete checks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does look like dead code. @kmahone to verify this codepath is not hit in any of the configurations we run tests in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Adds a check to only open a menubaritems flyout if it has items.
Motivation and Context
Closes #2740
How Has This Been Tested?
Tested manually:
Screenshots (if appropriate):