Skip to content
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

Layout Cycle Crash + Misc Fixes for NavigationView #5084

Merged
merged 22 commits into from
Jun 12, 2021
Merged

Layout Cycle Crash + Misc Fixes for NavigationView #5084

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
275cdc9
separator margn fix
karkarl May 24, 2021
e36718c
add border and cornerRadius
karkarl May 4, 2021
06f90d7
fix jumping borders
karkarl May 5, 2021
18b686a
fix pixel jumping issue
karkarl May 5, 2021
026ab45
tweak resources
karkarl May 24, 2021
1ee640f
set NavigationViewContentGridBorderThickness default to 0
karkarl May 24, 2021
4c7ed20
fix bad merge
karkarl May 24, 2021
c0ae763
add border to content when in topnav
karkarl May 25, 2021
24a3ad8
changes from comments
karkarl May 25, 2021
24c4ed4
update verification files
karkarl May 26, 2021
4b91172
fix pane behaviour testS
karkarl May 26, 2021
74d03fe
fix tests
karkarl Jun 4, 2021
642124d
refactor updatePaneLayout
karkarl Jun 10, 2021
c3c4811
clean up
karkarl Jun 10, 2021
db5b373
changes from comments
karkarl Jun 10, 2021
81ab023
add separator and margin top bottom to Height calculation
karkarl Jun 10, 2021
2b6a3ea
changes from comments
karkarl Jun 10, 2021
35bd4b0
changes from comments
karkarl Jun 11, 2021
34a930a
fix tests
karkarl Jun 11, 2021
30a4f0c
update verification files
karkarl Jun 11, 2021
c273364
revert separator default visiblity
karkarl Jun 11, 2021
c89a7a5
update verification files
karkarl Jun 11, 2021
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
49 changes: 30 additions & 19 deletions dev/NavigationView/NavigationView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ static constexpr auto c_paneHeaderContentBorderRow = L"PaneHeaderContentBorderRo

static constexpr auto c_separatorVisibleStateName = L"SeparatorVisible";
static constexpr auto c_separatorCollapsedStateName = L"SeparatorCollapsed";
static constexpr auto c_visualItemsSeparator = L"VisualItemsSeparator"sv;

static constexpr int c_backButtonHeight = 40;
static constexpr int c_backButtonWidth = 40;
Expand Down Expand Up @@ -658,7 +657,6 @@ void NavigationView::OnApplyTemplate()
m_itemsContainerRow.set(GetTemplateChildT<winrt::RowDefinition>(c_itemsContainerRow, controlProtected));
m_menuItemsScrollViewer.set(GetTemplateChildT<winrt::FrameworkElement>(c_menuItemsScrollViewer, controlProtected));
m_footerItemsScrollViewer.set(GetTemplateChildT<winrt::FrameworkElement>(c_footerItemsScrollViewer, controlProtected));
m_visualItemsSeparator.set(GetTemplateChildT<winrt::NavigationViewItemSeparator>(c_visualItemsSeparator, controlProtected));


m_itemsContainerSizeChangedRevoker.revoke();
Expand Down Expand Up @@ -1513,35 +1511,48 @@ void NavigationView::UpdatePaneLayout()
if (const auto& menuItems = m_leftNavRepeater.get())
{
const auto footersActualHeight = [this, footerItemsRepeater]() {
const auto footerItemsRepeaterMargin = footerItemsRepeater.Margin();
return footerItemsRepeater.ActualHeight() + footerItemsRepeaterMargin.Top + footerItemsRepeaterMargin.Bottom;
}();

const auto menuItemsActualHeight = [this, menuItems]() {
const auto menuItemsMargin = menuItems.Margin();
return menuItems.ActualHeight() + menuItemsMargin.Top + menuItemsMargin.Bottom;
double footerItemsRepeaterTopBottomMargin = 0.0;
if (footerItemsRepeater.Visibility() == winrt::Visibility::Visible)
{
const auto footerItemsRepeaterMargin = footerItemsRepeater.Margin();
footerItemsRepeaterTopBottomMargin = footerItemsRepeaterMargin.Top + footerItemsRepeaterMargin.Bottom;
}
return footerItemsRepeater.ActualHeight() + footerItemsRepeaterTopBottomMargin;
}();

const auto paneFooterActualHeight = [this]() {
if (const auto& paneFooter = m_leftNavFooterContentBorder.get())
{
const auto paneFooterMargin = paneFooter.Margin();
return paneFooter.ActualHeight() + paneFooterMargin.Top + paneFooterMargin.Bottom;
double paneFooterTopBottomMargin = 0.0;
if (paneFooter.Visibility() == winrt::Visibility::Visible)
{
const auto paneFooterMargin = paneFooter.Margin();
paneFooterTopBottomMargin = paneFooterMargin.Top + paneFooterMargin.Bottom;
}
return paneFooter.ActualHeight() + paneFooterTopBottomMargin;
}
return 0.0;
}();

const auto visualItemsSeparatorHeight = [this]() {
if (const auto& visualItemsSeparator = m_visualItemsSeparator.get())
const auto menuItemsTopBottomMargin = [this, menuItems]() {
if (menuItems.Visibility() == winrt::Visibility::Visible)
{
const auto visualItemsSeparatorMargin = visualItemsSeparator.Margin();
return visualItemsSeparator.ActualHeight() + visualItemsSeparatorMargin.Top + visualItemsSeparatorMargin.Bottom;
const auto menuItemsMargin = menuItems.Margin();
return menuItemsMargin.Top + menuItemsMargin.Bottom;
}
return 0.0;
}();

// Footer, PaneFooter, and VisualItemsSeparator are included in the footerGroup to calculate available height for menu items.
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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Unlike ActualHeight/ActualWidth, UIElement.DesiredSize already includes the Margin. So it should be:

const auto menuItemsDesiredHeight = menuItems.DesiredSize().Height;


// This is what the height ended up being, so will be the value that is used to calculate the partition
// between menuItems and footerGroup.
const auto menuItemsActualHeight = menuItems.ActualHeight() + menuItemsTopBottomMargin;

// Footer and PaneFooter are included in the footerGroup to calculate available height for menu items.
const auto footerGroupActualHeight = footersActualHeight + paneFooterActualHeight;

if (m_footerItemsSource.Count() == 0 && !IsSettingsVisible())
{
Expand All @@ -1554,14 +1565,14 @@ void NavigationView::UpdatePaneLayout()
winrt::VisualStateManager::GoToState(*this, c_separatorCollapsedStateName, false);
return 0.0;
}
else if (totalAvailableHeight > menuItemsActualHeight + footerGroupActualHeight)
else if (totalAvailableHeight >= menuItemsDesiredHeight + footerGroupActualHeight)
{
// We have enough space for two so let everyone get as much as they need.
footerItemsScrollViewer.MaxHeight(footersActualHeight);
winrt::VisualStateManager::GoToState(*this, c_separatorCollapsedStateName, false);
return totalAvailableHeight - footerGroupActualHeight;
}
else if (menuItemsActualHeight <= totalAvailableHeightHalf)
else if (menuItemsDesiredHeight <= totalAvailableHeightHalf)
{
// Footer items exceed over the half, so let's limit them.
footerItemsScrollViewer.MaxHeight(totalAvailableHeight - menuItemsActualHeight);
Expand Down
1 change: 0 additions & 1 deletion dev/NavigationView/NavigationView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,6 @@ class NavigationView :
tracker_ref<winrt::Border> m_topNavContentOverlayAreaGrid{ this };
tracker_ref<winrt::Grid> m_shadowCaster{ this };
tracker_ref<winrt::Storyboard> m_shadowCasterEaseOutStoryboard{ this };
tracker_ref<winrt::NavigationViewItemSeparator> m_visualItemsSeparator{ this };

// Indicator animations
tracker_ref<winrt::UIElement> m_prevIndicator{ this };
Expand Down