Layout Cycle Crash + Misc Fixes for NavigationView #5084
Conversation
ghost
added
the
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
ranjeshj
added
area-NavigationView
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
karkarl
changed the title
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| }(); | ||
|
|
||
| // Footer, PaneFooter, and VisualItemsSeparator are included in the footerGroup to calculate available height for menu items. | ||
| const auto footerGroupActualHeight = footersActualHeight + paneFooterActualHeight + visualItemsSeparatorHeight; |
There was a problem hiding this comment.
Adding the visualItemsSeparatorHeight feels incorrect to me: you don't want the separator to be shown because of a lack of space due to the separator being shown. You know what I mean?
There was a problem hiding this comment.
I thought about that too... but it was part of the calculation before so I simply included it.
There was a problem hiding this comment.
Discussed offline, will remove.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| const auto footerGroupActualHeight = footersActualHeight + paneFooterActualHeight + visualItemsSeparatorHeight; | ||
| // This is the value computed during the measure pass of the layout process. This will be the value used to determine | ||
| // the partition logic between menuItems and footerGroup, since the ActualHeight may be taller if there's more space. | ||
| const auto menuItemsDesiredHeight = menuItems.DesiredSize().Height + menuItemsTopBottomMargin; |
There was a problem hiding this comment.
Unlike ActualHeight/ActualWidth, UIElement.DesiredSize already includes the Margin. So it should be:
const auto menuItemsDesiredHeight = menuItems.DesiredSize().Height;
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| <VisualState x:Name="SeparatorVisible"/> | ||
| <VisualState x:Name="SeparatorCollapsed"> | ||
| <VisualState.Setters> | ||
| <Setter Target="VisualItemsSeparator.Visibility" Value="Visible"/> | ||
| <Setter Target="VisualItemsSeparator.Visibility" Value="Collapsed"/> |
There was a problem hiding this comment.
Out of curiosity, why is this switched now?
There was a problem hiding this comment.
oh oops. I was previously getting the separator ActualHeight from codebehind, and for it to evaluate to a non-zero value, it needed to be visible by default. The separator is no longer used for pane layout calculations, so I should revert this.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Description
Layout Cycle Crash Fix:
UpdatePaneLayout()was calculating Separator and Footer height from hardcoded values. The discrepancy from the template was causing ScrollViewer to continuously calculate incrementally different heights and crashing.It is now refactored to include all footer group items (
footerItemsRepeaterpaneFooter,VisualItemsSeparator) and its margins top and bottom.Miscellaneous fixes