Skip to content

Add check to only open flyout if a menubaritem has items #2889

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.

Sign up for GitHub

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

Merged
StephenLPeters merged 3 commits into from
Jul 17, 2020
Merged

Add check to only open flyout if a menubaritem has items #2889

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1672b33
Add check to only open flyout if a menubaritem has items
marcelwgn Jul 12, 2020
652aa8d
Add interaction test
marcelwgn Jul 13, 2020
4a03b7a
Fix failing test
marcelwgn Jul 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 23 additions & 20 deletions dev/MenuBar/MenuBarItem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,29 +214,32 @@ void MenuBarItem::OnMenuBarItemAccessKeyInvoked(winrt::IInspectable const& sende
// Menu Flyout actions
void MenuBarItem::ShowMenuFlyout()
{
if (auto button = m_button.get())
if (Items().Size() != 0)
{
const auto width = static_cast<float>(button.ActualWidth());
const auto height = static_cast<float>(button.ActualHeight());

if (SharedHelpers::IsFlyoutShowOptionsAvailable())
{
// Sets an exclusion rect over the button that generates the flyout so that even if the menu opens upwards
// (which is the default in touch mode) it doesn't cover the menu bar button.
winrt::FlyoutShowOptions options{};
options.Position(winrt::Point(0, height));
options.Placement(winrt::FlyoutPlacementMode::Bottom);
options.ExclusionRect(winrt::Rect(0, 0, width, height));
m_flyout.get().ShowAt(button, options);
}
else
if (auto button = m_button.get())
{
m_flyout.get().ShowAt(button, winrt::Point(0, height));
const auto width = static_cast<float>(button.ActualWidth());
const auto height = static_cast<float>(button.ActualHeight());

if (SharedHelpers::IsFlyoutShowOptionsAvailable())
{
// Sets an exclusion rect over the button that generates the flyout so that even if the menu opens upwards
// (which is the default in touch mode) it doesn't cover the menu bar button.
winrt::FlyoutShowOptions options{};
options.Position(winrt::Point(0, height));
options.Placement(winrt::FlyoutPlacementMode::Bottom);
options.ExclusionRect(winrt::Rect(0, 0, width, height));
m_flyout.get().ShowAt(button, options);
}
else
{
m_flyout.get().ShowAt(button, winrt::Point(0, height));
}

// Attach keyboard event handler
auto presenter = winrt::get_self<MenuBarItemFlyout>(m_flyout.get())->m_presenter.get();
m_presenterKeyDownRevoker = presenter.KeyDown(winrt::auto_revoke, { this, &MenuBarItem::OnPresenterKeyDown });
}

// Attach keyboard event handler
auto presenter = winrt::get_self<MenuBarItemFlyout>(m_flyout.get())->m_presenter.get();
m_presenterKeyDownRevoker = presenter.KeyDown(winrt::auto_revoke, { this, &MenuBarItem::OnPresenterKeyDown });
}
}

Expand Down
2 changes: 2 additions & 0 deletions dev/MenuBar/MenuBar_TestUI/MenuBarPage.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
<MenuFlyoutItem Text="Word Wrap"/>
<MenuFlyoutItem Text="Font..."/>
</muxc:MenuBarItem>

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

@ranjeshj ranjeshj Jul 13, 2020

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

@marcelwgn marcelwgn Jul 13, 2020

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.

</muxc:MenuBar>

<TextBlock Text="Custom sized menu bar" Margin="0,6,0,0"/>
Expand Down