NavigationView: Fixing visibility of close button in Auto PaneDisplayMode and Minimal DisplayMode #1195
Conversation
…Mode and Minimal DisplayMode for Issue #563.
jevansaks
added
area-NavigationView
| auto paneDisplayMode = PaneDisplayMode(); | ||
|
|
||
| if (paneDisplayMode != winrt::NavigationViewPaneDisplayMode::LeftMinimal && | ||
| (paneDisplayMode != winrt::NavigationViewPaneDisplayMode::Auto || DisplayMode() != winrt::NavigationViewDisplayMode::Minimal)) |
There was a problem hiding this comment.
What's the difference between paneDisplayMode and DisplayMode()?
Is NavigationViewDisplayMode::Minimal a sub-state of NavigationViewPaneDisplayMode::Auto? If so maybe something like below is easier to understand?
paneDisplayMode != winrt::NavigationViewPaneDisplayMode::LeftMinimal &&
!(paneDisplayMode == winrt::NavigationViewPaneDisplayMode::Auto && DisplayMode() == winrt::NavigationViewDisplayMode::Minimal)There was a problem hiding this comment.
Yulia told me this weird design was the result of feature additions during the lifespan of the control. PaneDisplayMode is read-write while DisplayMode is read-only. One PaneDisplayMode implies one of several DisplayMode values, depending on various factors. Minimal may be set for Auto and LeftMinimal. I'm sure this design would not have been picked if all today's features had existing from the beginning.
Another way of expressing the condition would have been:
if (!(paneDisplayMode == winrt::NavigationViewPaneDisplayMode::LeftMinimal ||
(paneDisplayMode == winrt::NavigationViewPaneDisplayMode::Auto && DisplayMode() == winrt::NavigationViewDisplayMode::Minimal)))
{
return false;
}
or your expression. None of them are trivial - I'll just leave it as is.
|
🎉 Handy links: |
Fixes Issue #563.
When the NavigationView pane is open, not only must the new Close button be displayed when PaneDisplayMode is LeftMinimal, it must also appear when the PaneDisplayMode is Auto and the DisplayMode is Minimal.
Only the NavigationView::ShouldShowCloseButton() method had to be tweaked to make that happen.
I added a new VerifyBackAndCloseButtonsVisibilityInAutoPaneDisplayMode test to cover that scenario.