Skip to content

Commit

Permalink
Fix crash using BackButtonBehavior and navigating to root (#11438) Fixes
Browse files Browse the repository at this point in the history
 #11401 Fixes #10514

* Fix crash using BackButtonBehavior and navigating to root

* Added Device Test

* - add unit test and fixup ShellToolbar to unhook from previous command

* - remove extra call to OnBackButtonCommandPropertyChanged

* - remove excess code from ShellToolbar

Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
  • Loading branch information
jsuarezruiz and PureWeen authored Dec 12, 2022
1 parent 45893ea commit 07951fa
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 37 deletions.
52 changes: 15 additions & 37 deletions src/Controls/src/Core/ShellToolbar.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
using System;
#nullable enable

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Runtime.CompilerServices;
using System.Text;
using System.Windows.Input;
Expand All @@ -10,8 +13,8 @@ namespace Microsoft.Maui.Controls
internal class ShellToolbar : Toolbar
{
Shell _shell;
Page _currentPage;
BackButtonBehavior _backButtonBehavior;
Page? _currentPage;
BackButtonBehavior? _backButtonBehavior;
ToolbarTracker _toolbarTracker = new ToolbarTracker();
#if WINDOWS
MenuBarTracker _menuBarTracker = new MenuBarTracker();
Expand Down Expand Up @@ -77,7 +80,7 @@ internal void ApplyChanges()
_menuBarTracker.Target = _shell;
#endif

Page previousPage = null;
Page? previousPage = null;
if (stack.Count > 1)
previousPage = stack[stack.Count - 1];

Expand All @@ -95,13 +98,6 @@ internal void ApplyChanges()
BackButtonVisible = backButtonVisible && stack.Count > 1;
BackButtonEnabled = _backButtonBehavior?.IsEnabled ?? true;

if (_backButtonBehavior?.Command != null)
{
BackButtonEnabled =
BackButtonEnabled &&
_backButtonBehavior.Command.CanExecute(_backButtonBehavior.CommandParameter);
}

UpdateTitle();

if (_currentPage != null &&
Expand Down Expand Up @@ -131,7 +127,6 @@ internal void ApplyChanges()
DynamicOverflowEnabled = PlatformConfiguration.WindowsSpecific.Page.GetToolbarDynamicOverflowEnabled(currentPage);
}

ICommand _backButtonCommand;
void UpdateBackbuttonBehavior()
{
var bbb = Shell.GetBackButtonBehavior(_currentPage);
Expand All @@ -140,37 +135,20 @@ void UpdateBackbuttonBehavior()
return;

if (_backButtonBehavior != null)
_backButtonBehavior.PropertyChanged -= OnBackButtonPropertyChanged;
_backButtonBehavior.PropertyChanged -= OnBackButtonCommandPropertyChanged;

_backButtonBehavior = bbb;

if (_backButtonBehavior != null)
_backButtonBehavior.PropertyChanged += OnBackButtonPropertyChanged;

void OnBackButtonPropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
{
ApplyChanges();

if (_backButtonBehavior.Command == _backButtonCommand)
return;

if (_backButtonCommand != null)
_backButtonCommand.CanExecuteChanged -= OnBackButtonCanExecuteChanged;

_backButtonCommand = _backButtonBehavior.Command;

if (_backButtonCommand != null)
_backButtonCommand.CanExecuteChanged += OnBackButtonCanExecuteChanged;
}
_backButtonBehavior.PropertyChanged += OnBackButtonCommandPropertyChanged;
}

void OnBackButtonCanExecuteChanged(object sender, EventArgs e)
{
BackButtonEnabled =
_backButtonCommand.CanExecute(_backButtonBehavior.CommandParameter);
}
void OnBackButtonCommandPropertyChanged(object? sender, PropertyChangedEventArgs e)
{
ApplyChanges();
}

void OnCurrentPagePropertyChanged(object sender, System.ComponentModel.PropertyChangedEventArgs e)
void OnCurrentPagePropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e)
{
if (e.Is(Page.TitleProperty))
UpdateTitle();
Expand All @@ -195,7 +173,7 @@ internal void UpdateTitle()
return;
}

Page currentPage = _shell.GetCurrentShellPage() as Page;
Page? currentPage = _shell.GetCurrentShellPage() as Page;
if (currentPage?.IsSet(Page.TitleProperty) == true)
{
Title = currentPage.Title ?? String.Empty;
Expand Down
44 changes: 44 additions & 0 deletions src/Controls/tests/Core.UnitTests/ShellToolbarTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,50 @@ public async Task BackButtonDisabledWhenCommandDisabled()
Assert.True(testShell.Toolbar.BackButtonEnabled);
}

[Fact]
public async Task BackButtonBehaviorCommandFromPoppedPageIsCorrectlyUnsubscribedFrom()
{
var firstPage = new ContentPage();
var secondPage = new ContentPage();
bool canExecute = true;
var backButtonBehavior = new BackButtonBehavior();
TestShell testShell = new TestShell(firstPage);

await testShell.Navigation.PushAsync(secondPage);

Shell.SetBackButtonBehavior(secondPage, backButtonBehavior);

backButtonBehavior.Command = new Command(() => { }, () => canExecute);

await testShell.Navigation.PopAsync();

canExecute = false;
(backButtonBehavior.Command as Command).ChangeCanExecute();

Assert.True(testShell.Toolbar.BackButtonEnabled);
}

[Fact]
public async Task BackButtonUpdatesWhenSetToNewCommand()
{
var firstPage = new ContentPage();
var secondPage = new ContentPage();
bool canExecute = true;
var backButtonBehavior = new BackButtonBehavior();
TestShell testShell = new TestShell(firstPage);

await testShell.Navigation.PushAsync(secondPage);

Shell.SetBackButtonBehavior(secondPage, backButtonBehavior);

backButtonBehavior.Command = new Command(() => { }, () => true);
Assert.True(testShell.Toolbar.BackButtonEnabled);
backButtonBehavior.Command = new Command(() => { }, () => false);
Assert.False(testShell.Toolbar.BackButtonEnabled);
backButtonBehavior.Command = null;
Assert.True(testShell.Toolbar.BackButtonEnabled);
}

[Fact]
public async Task ShellToolbarUpdatesFromNewBackButtonBehavior()
{
Expand Down
39 changes: 39 additions & 0 deletions src/Controls/tests/DeviceTests/Elements/Shell/ShellTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,45 @@ await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
});
}

[Fact(DisplayName = "Navigate to Root with BackButtonBehavior no Crash")]
public async Task NavigateToRootWithBackButtonBehaviorNoCrash()
{
SetupBuilder();

var shell = await CreateShellAsync(shell =>
{
shell.CurrentItem = new ContentPage();
});

await CreateHandlerAndAddToWindow<ShellHandler>(shell, async (handler) =>
{
Assert.False(IsBackButtonVisible(shell.Handler));
var secondPage = new ContentPage();
await shell.Navigation.PushAsync(secondPage);
Assert.True(IsBackButtonVisible(shell.Handler));
bool canExecute = true;
Command command = new Command(() => { }, () => canExecute);
BackButtonBehavior behavior = new BackButtonBehavior()
{
IsVisible = false,
Command = command
};
Shell.SetBackButtonBehavior(secondPage, behavior);
Assert.False(IsBackButtonVisible(shell.Handler));
behavior.IsVisible = true;
NavigationPage.SetHasBackButton(shell.CurrentPage, true);
await shell.Navigation.PopToRootAsync(false);
await Task.Delay(100);
canExecute = false;
command.ChangeCanExecute();
Assert.NotNull(shell.Handler);
});
}

protected Task<Shell> CreateShellAsync(Action<Shell> action) =>
InvokeOnMainThreadAsync(() =>
{
Expand Down

0 comments on commit 07951fa

Please sign in to comment.