Skip to content

Remove code that removed focus visual in SelectionFollowsFocus mode #3190

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
Sep 8, 2020
Merged

Remove code that removed focus visual in SelectionFollowsFocus mode #3190

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
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
23 changes: 0 additions & 23 deletions dev/NavigationView/NavigationView.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,6 @@ void NavigationView::OnApplyTemplate()
UpdatePaneTabFocusNavigation();
UpdateBackAndCloseButtonsVisibility();
UpdateSingleSelectionFollowsFocusTemplateSetting();
UpdateNavigationViewUseSystemVisual();
UpdatePaneVisibility();
UpdateVisualState();
UpdatePaneTitleMargins();
Expand Down Expand Up @@ -1135,11 +1134,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 @@ -3104,22 +3098,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)
Expand Down Expand Up @@ -3732,7 +3710,6 @@ void NavigationView::OnPropertyChanged(const winrt::DependencyPropertyChangedEve
else if (property == s_SelectionFollowsFocusProperty)
{
UpdateSingleSelectionFollowsFocusTemplateSetting();
UpdateNavigationViewUseSystemVisual();
}
else if (property == s_IsPaneToggleButtonVisibleProperty)
{
Expand Down
2 changes: 0 additions & 2 deletions dev/NavigationView/NavigationView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +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);
Copy link
Contributor

@StephenLPeters StephenLPeters Aug 31, 2020

Choose a reason for hiding this comment

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

PropagateShowFocusVisualToAllNavigationViewItemsInRepeater [](start = 16, length = 58)

I think this isn't called anymore?

Copy link
Collaborator Author

@marcelwgn marcelwgn Aug 31, 2020

Choose a reason for hiding this comment

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

Yes, this isn't used anymore, missed that function. Is removed now.

void UpdatePaneShadow();
void UpdateNavigationViewItemsFactory();
Expand Down Expand Up @@ -318,7 +317,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

@StephenLPeters StephenLPeters Aug 31, 2020

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

@marcelwgn marcelwgn Aug 31, 2020

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