Skip to content

Conversation

@marcelwgn
Copy link
Collaborator

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:

gif

Screenshots (if appropriate):

@msft-github-bot msft-github-bot added the needs-triage Issue needs to be triaged by the area owners label Jul 12, 2020
@ranjeshj ranjeshj added team-Controls Issue for the Controls team area-Menus labels Jul 13, 2020
<MenuFlyoutItem Text="Font..."/>
</muxc:MenuBarItem>

<muxc:MenuBarItem Title="No children"/>
Copy link
Contributor

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.

Copy link
Collaborator Author

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))
Copy link
Contributor

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.

Copy link
Collaborator Author

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.

Copy link
Contributor

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.

Copy link
Contributor

@StephenLPeters StephenLPeters left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters removed the needs-triage Issue needs to be triaged by the area owners label Jul 15, 2020
@StephenLPeters
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@StephenLPeters StephenLPeters merged commit 0a757bd into microsoft:master Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-Menus team-Controls Issue for the Controls team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MenuBarItem with no Items shows blank Dropdown

5 participants