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

Only show back button when changing from collapsed details view #941

Merged
merged 5 commits into from
May 21, 2017
+29 −28
Merged

Only show back button when changing from collapsed details view #941

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
022b9f4
remove property not used
skendrot Feb 15, 2017
4303ec2
create one method for common logic of updating view
skendrot Feb 15, 2017
9c82bbc
Only show the back button if in details mode or coming from details mode
skendrot Feb 15, 2017
09b563f
Show the back button only if it was shown when the control is rendered
skendrot Feb 15, 2017
3b3c767
set the previous visibility before setting the visibility when in det…
skendrot Feb 15, 2017
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
57 changes: 29 additions & 28 deletions Microsoft.Toolkit.Uwp.UI.Controls/MasterDetailsView/MasterDetailsView.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ public partial class MasterDetailsView : ItemsControl
private const string NoSelectionNarrowState = "NoSelectionNarrow";
private const string NoSelectionWideState = "NoSelectionWide";

private AppViewBackButtonVisibility _previousBackButtonVisibility;
private ContentPresenter _detailsPresenter;
private VisualStateGroup _stateGroup;
private VisualState _narrowState;
Expand Down Expand Up @@ -87,23 +88,16 @@ protected override void OnApplyTemplate()
private static void OnSelectedItemChanged(DependencyObject d, DependencyPropertyChangedEventArgs e)
{
var view = (MasterDetailsView)d;
if (view._stateGroup != null)
{
view.SetVisualState(view._stateGroup.CurrentState, true);
}

view.OnSelectionChanged(new SelectionChangedEventArgs(new List<object> { e.OldValue }, new List<object> { e.NewValue }));

view.UpdateView(true);

// If there is no selection, do not remove the DetailsPresenter content but let it animate out.
if (view.SelectedItem != null)
{
view.SetDetailsContent();
}

if (view._stateGroup != null)
{
view.SetBackButtonVisibility(view._stateGroup.CurrentState);
}
}

/// <summary>
Expand Down Expand Up @@ -142,10 +136,7 @@ private void OnLoaded(object sender, RoutedEventArgs e)

_narrowState = GetTemplateChild(NarrowState) as VisualState;

SetVisualState(_stateGroup.CurrentState, true);
SetBackButtonVisibility(_stateGroup.CurrentState);

UpdateViewState();
UpdateView(true);
}

private void OnUnloaded(object sender, RoutedEventArgs e)
Copy link

@ghost ghost Feb 15, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

Shouldn't the OnUnloaded method be extended with a method setting the BackButton to its previous state? Right now, when opening the details view and then navigating to another page, the back button remains visible (also in apps where the back button by default is not visible). If the user navigates back and the viewState is still Details, at that point it can be made visible again.

Expand All @@ -171,10 +162,7 @@ private void OnUnloaded(object sender, RoutedEventArgs e)
/// </remarks>
private void OnVisualStateChanged(object sender, VisualStateChangedEventArgs e)
{
SetBackButtonVisibility(e.NewState);

// When adaptive trigger changes state, switch between NoSelectionWide and NoSelectionNarrow.
SetVisualState(e.NewState, false);
UpdateView(false);
}

/// <summary>
Expand Down Expand Up @@ -216,30 +204,38 @@ private void SetMasterHeaderVisibility()
}
}

private void UpdateView(bool animate)
{
var currentState = ViewState;
UpdateViewState();
SetBackButtonVisibility(currentState);
if (_stateGroup != null)
{
SetVisualState(_stateGroup.CurrentState, animate);
}
}

/// <summary>
/// Sets the back button visibility based on the current visual state and selected item
/// </summary>
private void SetBackButtonVisibility(VisualState currentState)
private void SetBackButtonVisibility(MasterDetailsViewState previousState)
{
UpdateViewState();
if (DesignMode.DesignModeEnabled)
{
return;
}

if (ViewState == MasterDetailsViewState.Details)
{
SystemNavigationManager.GetForCurrentView().AppViewBackButtonVisibility =
AppViewBackButtonVisibility.Visible;
var navigationManager = SystemNavigationManager.GetForCurrentView();
_previousBackButtonVisibility = navigationManager.AppViewBackButtonVisibility;

navigationManager.AppViewBackButtonVisibility = AppViewBackButtonVisibility.Visible;
}
else
else if (previousState == MasterDetailsViewState.Details)
{
// Make sure we show the back button if the stack can navigate back
Copy link

@ghost ghost Feb 15, 2017 •
edited by ghost
Loading

Choose a reason for hiding this comment

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

This comment does not accurately describe the code block anymore. Could be updated or removed.

Personally I think some explanation can be helpful here; the same explanation you gave in your PR comment can be very insightful.

var frame = GetFrame();
SystemNavigationManager.GetForCurrentView().AppViewBackButtonVisibility =
((frame != null) && frame.CanGoBack)
? AppViewBackButtonVisibility.Visible
: AppViewBackButtonVisibility.Collapsed;
SystemNavigationManager.GetForCurrentView().AppViewBackButtonVisibility = _previousBackButtonVisibility;
}
}

Expand All @@ -250,6 +246,11 @@ private Frame GetFrame()

private void UpdateViewState()
{
if (_stateGroup == null)
{
return;
}

var before = ViewState;

if (_stateGroup.CurrentState == _narrowState || _stateGroup.CurrentState == null)
Expand Down Expand Up @@ -287,4 +288,4 @@ private void SetDetailsContent()
}
}
}
}
}