Skip to content

Commit

Permalink
[Windows] Fix Shell animation and button visual states on Shell nav (#…
Browse files Browse the repository at this point in the history
…26521)

* * Ensure pointer release event is fired on button unload
* Clear previous page content presenter on unload to prevent brief visual flash

* Add test

* Fix test name

* Add fix for ImageButton

* * Ensure pointer release event is fired on button unload
* Clear previous page content presenter on unload to prevent brief visual flash

* Add test

* Fix test name

* Add fix for ImageButton

* Adjust logic for content presenter unload

* Fix for dupe code

* Fix touch/click/press down event

* Remove commented code

---------

Co-authored-by: Mike Corsaro <mikecorsaro@microsoft.com>
  • Loading branch information
Foda and Mike Corsaro authored Jan 10, 2025
1 parent 70cc3dd commit 786b3aa
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 5 deletions.
36 changes: 36 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue26466.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.Github, 26466, "Button is not released on unload", PlatformAffected.WinRT)]
public partial class Issue26466 : TestContentPage
{
protected override void Init()
{
var button = new Button()
{
Text = "Hello",
AutomationId = "thebutton"
};

var success = new Label
{
Text = "If you see this, the test has passed",
AutomationId = "success"
};

var layout = new Microsoft.Maui.Controls.StackLayout();
layout.Children.Add(button);

button.Pressed += (s, e) =>
{
layout.Children.Remove(button);
};

button.Released += (s, e) =>
{
layout.Children.Add(success);
};

Content = layout;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#if WINDOWS
using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues
{
public class Issue26466 : _IssuesUITest
{
const string ButtonId = "thebutton";
const string SuccessId = "success";

public Issue26466(TestDevice testDevice) : base(testDevice)
{
}

public override string Issue => "Button is not released on unload";

[Test]
[Category(UITestCategories.Button)]
public async Task ButtonReleasedTest()
{
App.WaitForElement(ButtonId);
App.PressDown(ButtonId);

await Task.Delay(200);

// Button should be unloaded, so we should see the success label
App.WaitForElement(SuccessId);
}
}
}
#endif
15 changes: 15 additions & 0 deletions src/Core/src/Handlers/Button/ButtonHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public partial class ButtonHandler : ViewHandler<IButton, Button>
{
PointerEventHandler? _pointerPressedHandler;
PointerEventHandler? _pointerReleasedHandler;
bool _isPressed;

protected override Button CreatePlatformView() => new MauiButton();

Expand All @@ -18,15 +19,18 @@ protected override void ConnectHandler(Button platformView)
_pointerReleasedHandler = new PointerEventHandler(OnPointerReleased);

platformView.Click += OnClick;
platformView.Unloaded += OnUnloaded;
platformView.AddHandler(UIElement.PointerPressedEvent, _pointerPressedHandler, true);
platformView.AddHandler(UIElement.PointerReleasedEvent, _pointerReleasedHandler, true);

base.ConnectHandler(platformView);
}


protected override void DisconnectHandler(Button platformView)
{
platformView.Click -= OnClick;
platformView.Unloaded -= OnUnloaded;
platformView.RemoveHandler(UIElement.PointerPressedEvent, _pointerPressedHandler);
platformView.RemoveHandler(UIElement.PointerReleasedEvent, _pointerReleasedHandler);

Expand Down Expand Up @@ -97,14 +101,25 @@ void OnClick(object sender, RoutedEventArgs e)

void OnPointerPressed(object sender, PointerRoutedEventArgs e)
{
_isPressed = true;
VirtualView?.Pressed();
}

void OnPointerReleased(object sender, PointerRoutedEventArgs e)
{
_isPressed = false;
VirtualView?.Released();
}

void OnUnloaded(object sender, RoutedEventArgs e)
{
// WinUI will not raise the PointerReleased event if the pointer is pressed and then unloaded
if (_isPressed)
{
VirtualView?.Released();
}
}

partial class ButtonImageSourcePartSetter
{
public override void SetImageSource(ImageSource? platformImage)
Expand Down
14 changes: 14 additions & 0 deletions src/Core/src/Handlers/ImageButton/ImageButtonHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ public partial class ImageButtonHandler : ViewHandler<IImageButton, Button>

PointerEventHandler? _pointerPressedHandler;
PointerEventHandler? _pointerReleasedHandler;
bool _isPressed;

protected override Button CreatePlatformView()
{
Expand Down Expand Up @@ -44,6 +45,7 @@ protected override void ConnectHandler(Button platformView)
}

platformView.Click += OnClick;
platformView.Unloaded += OnUnloaded;
platformView.AddHandler(UIElement.PointerPressedEvent, _pointerPressedHandler, true);
platformView.AddHandler(UIElement.PointerReleasedEvent, _pointerReleasedHandler, true);

Expand All @@ -59,6 +61,7 @@ protected override void DisconnectHandler(Button platformView)
}

platformView.Click -= OnClick;
platformView.Unloaded -= OnUnloaded;
platformView.RemoveHandler(UIElement.PointerPressedEvent, _pointerPressedHandler);
platformView.RemoveHandler(UIElement.PointerReleasedEvent, _pointerReleasedHandler);

Expand Down Expand Up @@ -104,14 +107,25 @@ void OnClick(object sender, RoutedEventArgs e)

void OnPointerPressed(object sender, PointerRoutedEventArgs e)
{
_isPressed = true;
VirtualView?.Pressed();
}

void OnPointerReleased(object sender, PointerRoutedEventArgs e)
{
_isPressed = false;
VirtualView?.Released();
}

void OnUnloaded(object sender, RoutedEventArgs e)
{
// WinUI will not raise the PointerReleased event if the pointer is pressed and then unloaded
if (_isPressed)
{
VirtualView?.Released();
}
}

void OnImageOpened(object sender, RoutedEventArgs routedEventArgs)
{
VirtualView?.UpdateIsLoading(false);
Expand Down
35 changes: 32 additions & 3 deletions src/Core/src/Platform/Windows/StackNavigationManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,16 @@ public virtual void Connect(IStackNavigation navigationView, Frame navigationFra
{
_connected = true;
if (_navigationFrame != null)
{
_navigationFrame.Navigating -= OnNavigating;
_navigationFrame.Navigated -= OnNavigated;

}

FirePendingNavigationFinished();

navigationFrame.Navigating += OnNavigating;
navigationFrame.Navigated += OnNavigated;

_navigationFrame = navigationFrame;
NavigationView = (IStackNavigation)navigationView;

Expand All @@ -51,7 +56,10 @@ public virtual void Disconnect(IStackNavigation navigationView, Frame navigation
{
_connected = false;
if (_navigationFrame != null)
{
_navigationFrame.Navigating -= OnNavigating;
_navigationFrame.Navigated -= OnNavigated;
}

FirePendingNavigationFinished();
_navigationFrame = null;
Expand Down Expand Up @@ -143,6 +151,28 @@ void SyncBackStackToNavigationStack(IReadOnlyList<IView> pageStack)
}
}

private void OnNavigating(object sender, NavigatingCancelEventArgs e)
{
// We can navigate to pages that do not exist in the shell as sections, so we
// end up keeping the previous page loaded and in the (MAUI) visual tree.
// This means we need to manually clear the WinUI content from the presenter so we don't
// get a crash when the page is navigated to again due to the content already being the
// child of another parent
if (NavigationFrame.Content is Page p)
{
p.Unloaded += PageUnloaded;
void PageUnloaded(object s, RoutedEventArgs e)
{
p.Unloaded -= PageUnloaded;
if (p.Content is ContentPresenter presenter)
{
presenter.Content = null;
_previousContent = null;
}
}
}
}

// This is used to fire NavigationFinished back to the xplat view
// Firing NavigationFinished from Loaded is the latest reliable point
// in time that I know of for firing `NavigationFinished`
Expand Down Expand Up @@ -170,8 +200,7 @@ void OnNavigated(object sender, UI.Xaml.Navigation.NavigationEventArgs e)
VerticalAlignment = UI.Xaml.VerticalAlignment.Stretch
};

// There's some bug in our code, or the lifecycle of ContentControl, that is causing the content to
// never be removed from the parent...
// Clear the content just in case the previous page didn't unload
if (_previousContent is not null)
{
_previousContent.Content = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class AppiumMouseActions : ICommandExecutionGroup
ClickCoordinatesCommand,
DoubleClickCommand,
DoubleClickCoordinatesCommand,
LongPressCommand,
LongPressCommand
};

public AppiumMouseActions(AppiumApp appiumApp)
Expand Down
21 changes: 20 additions & 1 deletion src/TestUtils/src/UITest.Appium/Actions/AppiumTouchActions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ public class AppiumTouchActions : ICommandExecutionGroup
const string DragAndDropCommand = "dragAndDrop";
const string ScrollToCommand = "scrollTo";
const string DragCoordinatesCommand = "dragCoordinates";
const string PressDownCommand = "pressDown";

readonly AppiumApp _appiumApp;

Expand All @@ -32,7 +33,8 @@ public class AppiumTouchActions : ICommandExecutionGroup
TouchAndHoldCoordinatesCommand,
DragAndDropCommand,
ScrollToCommand,
DragCoordinatesCommand
DragCoordinatesCommand,
PressDownCommand
};

public AppiumTouchActions(AppiumApp appiumApp)
Expand All @@ -58,6 +60,8 @@ public CommandResponse Execute(string commandName, IDictionary<string, object> p
DragAndDropCommand => DragAndDrop(parameters),
ScrollToCommand => ScrollTo(parameters),
DragCoordinatesCommand => DragCoordinates(parameters),

PressDownCommand => PressDown(parameters),
_ => CommandResponse.FailedEmptyResponse,
};
}
Expand Down Expand Up @@ -132,6 +136,21 @@ CommandResponse TapCoordinates(float x, float y)
return CommandResponse.SuccessEmptyResponse;
}

CommandResponse PressDown(IDictionary<string, object> parameters)
{
var element = GetAppiumElement(parameters["element"]);

// Currently only pen and touch pointer input source types are supported, but this works fine
OpenQA.Selenium.Appium.Interactions.PointerInputDevice touchDevice = new OpenQA.Selenium.Appium.Interactions.PointerInputDevice(PointerKind.Touch);

var sequence = new ActionSequence(touchDevice, 0);
sequence.AddAction(touchDevice.CreatePointerMove(element, 0, 0, TimeSpan.FromMilliseconds(5)));
sequence.AddAction(touchDevice.CreatePointerDown(PointerButton.TouchContact));
sequence.AddAction(touchDevice.CreatePause(TimeSpan.FromMilliseconds(250)));
_appiumApp.Driver.PerformActions(new List<ActionSequence> { sequence });

return CommandResponse.SuccessEmptyResponse;
}

CommandResponse RightClick(string elementId)
{
Expand Down
14 changes: 14 additions & 0 deletions src/TestUtils/src/UITest.Appium/HelperExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,20 @@ public static void RightClick(this IApp app, string element)
});
}

/// <summary>
/// Performs a down/press on the matched element, without a matching release
/// </summary>
/// <param name="app"></param>
/// <param name="element"></param>
public static void PressDown(this IApp app, string element)
{
var uiElement = FindElement(app, element);
uiElement.Command.Execute("pressDown", new Dictionary<string, object>()
{
{ "element", uiElement }
});
}

public static string? GetText(this IUIElement element)
{
var response = element.Command.Execute("getText", new Dictionary<string, object>()
Expand Down

0 comments on commit 786b3aa

Please sign in to comment.