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

Remove code that removed focus visual in SelectionFollowsFocus mode #3190

Merged
merged 3 commits into from
Sep 8, 2020
Merged

Remove code that removed focus visual in SelectionFollowsFocus mode #3190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3356e8c
Remove code that removed focus visual in SelectionFollowsFocus mode
marcelwgn Aug 24, 2020
d3947d7
Remove unused func
marcelwgn Aug 31, 2020
5f20943
Merge branch 'master' into user/chingucoding/navview-focusindicator-s…
marcelwgn Sep 3, 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
46 changes: 0 additions & 46 deletions dev/NavigationView/NavigationView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,6 @@ void NavigationView::OnApplyTemplate()
UpdatePaneTabFocusNavigation();
UpdateBackAndCloseButtonsVisibility();
UpdateSingleSelectionFollowsFocusTemplateSetting();
UpdateNavigationViewUseSystemVisual();
UpdatePaneVisibility();
UpdateVisualState();
UpdatePaneTitleMargins();
Expand Down Expand Up @@ -1141,11 +1140,6 @@ void NavigationView::OnRepeaterElementPrepared(const winrt::ItemsRepeater& ir, c
}();
winrt::get_self<NavigationViewItem>(nvi)->PropagateDepthToChildren(childDepth);

if (ir != m_topNavRepeaterOverflowView.get())
{
nvibImpl->UseSystemFocusVisuals(ShouldShowFocusVisual());
}

// Register for item events
auto nviRevokers = winrt::make_self<NavigationViewItemRevokers>();
nviRevokers->tappedRevoker = nvi.Tapped(winrt::auto_revoke, { this, &NavigationView::OnNavigationViewItemTapped });
Expand Down Expand Up @@ -3121,45 +3115,6 @@ void NavigationView::UpdateLeftNavigationOnlyVisualState(bool useTransitions)
winrt::VisualStateManager::GoToState(*this, isToggleButtonVisible ? L"TogglePaneButtonVisible" : L"TogglePaneButtonCollapsed", false /*useTransitions*/);
}

void NavigationView::UpdateNavigationViewUseSystemVisual()
{
if (SharedHelpers::IsRS1OrHigher() && !ShouldPreserveNavigationViewRS4Behavior() && m_appliedTemplate)
{
PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(m_leftNavRepeater.get(), ShouldShowFocusVisual());
PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(m_leftNavFooterMenuRepeater.get(), ShouldShowFocusVisual());
PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(m_topNavRepeater.get(), ShouldShowFocusVisual());
PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(m_topNavFooterMenuRepeater.get(), ShouldShowFocusVisual());
}
}

bool NavigationView::ShouldShowFocusVisual()
{
return SelectionFollowsFocus() == winrt::NavigationViewSelectionFollowsFocus::Disabled;
}

void NavigationView::PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(winrt::ItemsRepeater const& ir, bool showFocusVisual)
{
if (ir)
{
if (auto itemsSourceView = ir.ItemsSourceView())
{
const auto numberOfItems = itemsSourceView.Count();
for (int i = 0; i < numberOfItems; i++)
{
if (auto nvib = ir.TryGetElement(i))
{
if (auto nvi = nvib.try_as<winrt::NavigationViewItem>())
{
auto nviImpl = winrt::get_self<NavigationViewItem>(nvi);
nviImpl->UseSystemFocusVisuals(showFocusVisual);
}
}

}
}
}
}

void NavigationView::InvalidateTopNavPrimaryLayout()
{
if (m_appliedTemplate && IsTopNavigationView())
Expand Down Expand Up @@ -3749,7 +3704,6 @@ void NavigationView::OnPropertyChanged(const winrt::DependencyPropertyChangedEve
else if (property == s_SelectionFollowsFocusProperty)
{
UpdateSingleSelectionFollowsFocusTemplateSetting();
UpdateNavigationViewUseSystemVisual();
}
else if (property == s_IsPaneToggleButtonVisibleProperty)
{
Expand Down
3 changes: 0 additions & 3 deletions dev/NavigationView/NavigationView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,6 @@ class NavigationView :
void UpdateVisualState(bool useTransitions = false);
void UpdateVisualStateForOverflowButton();
void UpdateLeftNavigationOnlyVisualState(bool useTransitions);
void UpdateNavigationViewUseSystemVisual();
static void PropagateShowFocusVisualToAllNavigationViewItemsInRepeater(winrt::ItemsRepeater const& ir, bool showFocusVisual);
void UpdatePaneShadow();
void UpdateNavigationViewItemsFactory();
void SyncItemTemplates();
Expand Down Expand Up @@ -318,7 +316,6 @@ class NavigationView :
bool ShouldPreserveNavigationViewRS3Behavior();

bool NeedRearrangeOfTopElementsAfterOverflowSelectionChanged(int selectedOriginalIndex);
bool ShouldShowFocusVisual();
void KeyboardFocusFirstItemFromItem(const winrt::NavigationViewItemBase& nvib);
void KeyboardFocusLastItemFromItem(const winrt::NavigationViewItemBase& nvib);
void FocusNextDownItem(const winrt::NavigationViewItem& nvi, const winrt::KeyRoutedEventArgs& args);
Expand Down
4 changes: 0 additions & 4 deletions dev/NavigationView/NavigationView.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,10 +508,6 @@
</Setter>
</Style>

<!--Note: Even though UseSystemFocusVisuals is set to True here in the Style, and because
bindings in Styles are only evaluated once, we were unable to bring a TemplateSettings.UseSystemFocusVisuals
Copy link
Contributor

Choose a reason for hiding this comment

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

bindings in Styles are only evaluated once [](start = 4, length = 42)

@chingucoding and @Felix-Dev does this tid bit explain some of the Binding issues you've been seeing in other PRs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That certainly explains the issues I have seen. So the workaround is to essentially handle the binding inside the control logic then?

to handle the removal of the focus rect when SelectionFollowsFocus is enabled. Therefore, that has
been done in code and it's updated whenever SelectionFollowsFocus is changed.-->
<Style TargetType="local:NavigationViewItem">
<Setter Property="Foreground" Value="{ThemeResource NavigationViewItemForeground}" />
<Setter Property="Background" Value="{ThemeResource NavigationViewItemBackground}" />
Expand Down