diff --git a/src/Controls/src/Core/SwipeView/SwipeItem.cs b/src/Controls/src/Core/SwipeView/SwipeItem.cs index c295bc49362d..a314d2c83570 100644 --- a/src/Controls/src/Core/SwipeView/SwipeItem.cs +++ b/src/Controls/src/Core/SwipeView/SwipeItem.cs @@ -14,7 +14,7 @@ public partial class SwipeItem : MenuItem, Controls.ISwipeItem, Maui.ISwipeItemM public static readonly BindableProperty BackgroundColorProperty = BindableProperty.Create(nameof(BackgroundColor), typeof(Color), typeof(SwipeItem), null); /// Bindable property for . - public static readonly BindableProperty IsVisibleProperty = BindableProperty.Create(nameof(IsVisible), typeof(bool), typeof(SwipeItem), true); + public static readonly BindableProperty IsVisibleProperty = BindableProperty.Create(nameof(IsVisible), typeof(bool), typeof(SwipeItem), true, propertyChanged: OnIsVisibleChanged); /// /// Gets or sets the background color of the swipe item. This is a bindable property. @@ -36,6 +36,11 @@ public bool IsVisible public event EventHandler Invoked; + static void OnIsVisibleChanged(BindableObject bindable, object oldValue, object newValue) + { + var swipeItem = (SwipeItem)bindable; + swipeItem.Handler?.UpdateValue(nameof(ISwipeItemMenuItem.Visibility)); + } Paint ISwipeItemMenuItem.Background => new SolidPaint(BackgroundColor); Visibility ISwipeItemMenuItem.Visibility => this.IsVisible ? Visibility.Visible : Visibility.Collapsed; diff --git a/src/Controls/tests/TestCases.HostApp/Issues/Issue35216.cs b/src/Controls/tests/TestCases.HostApp/Issues/Issue35216.cs new file mode 100644 index 000000000000..2efc2cea608b --- /dev/null +++ b/src/Controls/tests/TestCases.HostApp/Issues/Issue35216.cs @@ -0,0 +1,100 @@ +using System.ComponentModel; +using System.Runtime.CompilerServices; + +namespace Maui.Controls.Sample.Issues; + +[Issue(IssueTracker.Github, 35216, "SwipeItem IsVisible should properly refresh native swipe items when binding value changes dynamically", PlatformAffected.UWP)] +public class Issue35216 : ContentPage +{ + readonly Issue35216ViewModel _viewModel = new() { IsDeleteVisible = false }; + SwipeView _swipeView; + + public Issue35216() + { + BindingContext = _viewModel; + + SwipeItem deleteSwipeItem = new SwipeItem + { + Text = "Delete", + BackgroundColor = Colors.Green, + AutomationId = "DeleteSwipeItem" + }; + deleteSwipeItem.SetBinding(SwipeItem.IsVisibleProperty, new Binding(nameof(Issue35216ViewModel.IsDeleteVisible))); + + SwipeItem archiveSwipeItem = new SwipeItem + { + Text = "Archive", + BackgroundColor = Colors.Blue, + AutomationId = "ArchiveSwipeItem" + }; + + _swipeView = new SwipeView + { + HeightRequest = 60, + LeftItems = new SwipeItems { deleteSwipeItem, archiveSwipeItem }, + Content = new Grid + { + BackgroundColor = Colors.LightGray, + Children = + { + new Label + { + AutomationId = "SwipeContent", + Text = "Swipe right to reveal items", + HorizontalOptions = LayoutOptions.Center, + VerticalOptions = LayoutOptions.Center + } + } + } + }; + + Button toggleButton = new Button + { + Text = "Toggle Delete Visibility", + AutomationId = "ToggleVisibilityButton" + }; + toggleButton.Clicked += (s, e) => _viewModel.IsDeleteVisible = !_viewModel.IsDeleteVisible; + + Button resetButton = new Button + { + Text = "Reset", + AutomationId = "ResetButton" + }; + resetButton.Clicked += (s, e) => _viewModel.IsDeleteVisible = false; + + Content = new VerticalStackLayout + { + Padding = new Thickness(20), + Spacing = 20, + Children = + { + _swipeView, + toggleButton, + resetButton, + } + }; + } +} + +public class Issue35216ViewModel : INotifyPropertyChanged +{ + bool _isDeleteVisible; + + public bool IsDeleteVisible + { + get => _isDeleteVisible; + set + { + if (_isDeleteVisible != value) + { + _isDeleteVisible = value; + OnPropertyChanged(); + } + } + } + + public event PropertyChangedEventHandler PropertyChanged; + + protected void OnPropertyChanged([CallerMemberName] string name = null) => + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(name)); +} diff --git a/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35216.cs b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35216.cs new file mode 100644 index 000000000000..5e4d6842fc1d --- /dev/null +++ b/src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue35216.cs @@ -0,0 +1,75 @@ +#if WINDOWS // Existing PR for iOS & Android: https://github.com/dotnet/maui/pull/35217 +using NUnit.Framework; +using UITest.Appium; +using UITest.Core; + +namespace Microsoft.Maui.TestCases.Tests.Issues; + +public class Issue35216 : _IssuesUITest +{ + public override string Issue => "SwipeItem IsVisible should properly refresh native swipe items when binding value changes dynamically"; + + public Issue35216(TestDevice device) : base(device) + { + } + + [Test] + [Order(1)] + [Category(UITestCategories.SwipeView)] + public void Issue35216SwipeItemInitiallyHiddenBecomesVisibleAfterBindingChanges() + { + Exception? exception = null; + var rect = App.WaitForElement("SwipeContent").GetRect(); + var centerX = rect.X + rect.Width / 2; + var centerY = rect.Y + rect.Height / 2; + + App.DragCoordinates(centerX, centerY, centerX + 200, centerY); + + VerifyScreenshotOrSetException(ref exception, "Issue35216SwipeOpen_InitiallyHidden", + retryTimeout: TimeSpan.FromSeconds(2), tolerance: 1.0); + + App.Tap("ToggleVisibilityButton"); + + App.DragCoordinates(centerX, centerY, centerX + 200, centerY); + VerifyScreenshotOrSetException(ref exception, "Issue35216SwipeOpen_BecomeVisible", + retryTimeout: TimeSpan.FromSeconds(2), tolerance: 1.0); + + App.Tap("ResetButton"); + + if (exception is not null) + { + throw exception; + } + } + + [Test] + [Order(2)] + [Category(UITestCategories.SwipeView)] + public void Issue35216SwipeItemBecomesHiddenAfterBindingChanges() + { + Exception? exception = null; + var rect = App.WaitForElement("SwipeContent").GetRect(); + var centerX = rect.X + rect.Width / 2; + var centerY = rect.Y + rect.Height / 2; + + App.Tap("ToggleVisibilityButton"); + App.DragCoordinates(centerX, centerY, centerX + 200, centerY); + + VerifyScreenshotOrSetException(ref exception, "Issue35216SwipeOpen_DeleteVisible", + retryTimeout: TimeSpan.FromSeconds(2), tolerance: 1.0); + + App.Tap("ToggleVisibilityButton"); + + App.DragCoordinates(centerX, centerY, centerX + 200, centerY); + VerifyScreenshotOrSetException(ref exception, "Issue35216SwipeOpen_DeleteHidden", + retryTimeout: TimeSpan.FromSeconds(2), tolerance: 1.0); + + App.Tap("ResetButton"); + + if (exception is not null) + { + throw exception; + } + } +} +#endif diff --git a/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_BecomeVisible.png b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_BecomeVisible.png new file mode 100644 index 000000000000..abb6e13951f2 Binary files /dev/null and b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_BecomeVisible.png differ diff --git a/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteHidden.png b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteHidden.png new file mode 100644 index 000000000000..dfe6142c1a3a Binary files /dev/null and b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteHidden.png differ diff --git a/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteVisible.png b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteVisible.png new file mode 100644 index 000000000000..80ce7ad55da8 Binary files /dev/null and b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_DeleteVisible.png differ diff --git a/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_InitiallyHidden.png b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_InitiallyHidden.png new file mode 100644 index 000000000000..58f22c47bfca Binary files /dev/null and b/src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/Issue35216SwipeOpen_InitiallyHidden.png differ diff --git a/src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs b/src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs index 3a9a33b6791d..aca3d0ddb087 100644 --- a/src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs +++ b/src/Core/src/Handlers/SwipeItemMenuItem/SwipeItemMenuItemHandler.Windows.cs @@ -28,7 +28,48 @@ public static void MapText(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem public static void MapBackground(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem view) => handler.PlatformView.UpdateBackground(view.Background); - public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem view) { } + public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem view) + { + // WinUI SwipeItem does not support a Visibility property, so we need to + // rebuild the parent SwipeView's swipe items to reflect the visibility change. + var swipeView = GetParentSwipeView(view); + if (swipeView?.Handler is ISwipeViewHandler swipeViewHandler) + { + if (swipeView.LeftItems?.Contains(view) == true) + { + swipeViewHandler.UpdateValue(nameof(ISwipeView.LeftItems)); + } + else if (swipeView.RightItems?.Contains(view) == true) + { + swipeViewHandler.UpdateValue(nameof(ISwipeView.RightItems)); + } + else if (swipeView.TopItems?.Contains(view) == true) + { + swipeViewHandler.UpdateValue(nameof(ISwipeView.TopItems)); + } + else if (swipeView.BottomItems?.Contains(view) == true) + { + swipeViewHandler.UpdateValue(nameof(ISwipeView.BottomItems)); + } + } + } + + // Walk up the virtual view parent chain to find the owning ISwipeView. + // This is more robust than assuming a fixed depth (Parent?.Parent) which + // can silently fail if parenting is in transition. + static ISwipeView? GetParentSwipeView(IElement? element) + { + var parent = element?.Parent; + while (parent is not null) + { + if (parent is ISwipeView swipeView) + { + return swipeView; + } + parent = parent.Parent; + } + return null; + } protected override void ConnectHandler(WSwipeItem platformView) { diff --git a/src/Core/src/Handlers/SwipeView/SwipeViewHandler.Windows.cs b/src/Core/src/Handlers/SwipeView/SwipeViewHandler.Windows.cs index fea1902e2dae..76e4ff745251 100644 --- a/src/Core/src/Handlers/SwipeView/SwipeViewHandler.Windows.cs +++ b/src/Core/src/Handlers/SwipeView/SwipeViewHandler.Windows.cs @@ -9,6 +9,9 @@ namespace Microsoft.Maui.Handlers { public partial class SwipeViewHandler : ViewHandler { + // Guard flag to prevent re-entrancy when CreateSwipeItems calls item.ToHandler(), + // which triggers MapVisibility, which calls UpdateValue back into MapLeftItems/MapRightItems. + bool _isRebuildingSwipeItems; protected override WSwipeControl CreatePlatformView() => new(); public override void SetVirtualView(IView view) @@ -83,11 +86,21 @@ public static void MapLeftItems(ISwipeViewHandler handler, ISwipeView view) if (!handler.PlatformView.IsLoaded) return; + if (handler is SwipeViewHandler { _isRebuildingSwipeItems: true }) + { + return; + } + UpdateSwipeItems(SwipeDirection.Left, handler, view, (items) => handler.PlatformView.LeftItems = items, view.LeftItems, handler.PlatformView.LeftItems); } public static void MapTopItems(ISwipeViewHandler handler, ISwipeView view) { + if (handler is SwipeViewHandler { _isRebuildingSwipeItems: true }) + { + return; + } + UpdateSwipeItems(SwipeDirection.Up, handler, view, (items) => handler.PlatformView.TopItems = items, view.TopItems, handler.PlatformView.TopItems); } @@ -96,11 +109,21 @@ public static void MapRightItems(ISwipeViewHandler handler, ISwipeView view) if (!handler.PlatformView.IsLoaded) return; + if (handler is SwipeViewHandler { _isRebuildingSwipeItems: true }) + { + return; + } + UpdateSwipeItems(SwipeDirection.Right, handler, view, (items) => handler.PlatformView.RightItems = items, view.RightItems, handler.PlatformView.RightItems); } public static void MapBottomItems(ISwipeViewHandler handler, ISwipeView view) { + if (handler is SwipeViewHandler { _isRebuildingSwipeItems: true }) + { + return; + } + UpdateSwipeItems(SwipeDirection.Down, handler, view, (items) => handler.PlatformView.BottomItems = items, view.BottomItems, handler.PlatformView.BottomItems); } @@ -178,13 +201,40 @@ static WSwipeItems CreateSwipeItems(SwipeDirection swipeDirection, ISwipeViewHan swipeItems.Mode = items.Mode.ToPlatform(); - foreach (var item in items) + // Set the re-entrancy guard before calling ToHandler() on each item. + // ToHandler() triggers initial property mapping, which calls MapVisibility, + // which calls UpdateValue back into MapLeftItems/MapRightItems — causing N + // redundant rebuilds. The guard prevents those nested calls from re-entering. + if (handler is SwipeViewHandler concreteHandler) { - if (CanAddSwipeItems(swipeItems) && item is ISwipeItemMenuItem && - item.ToHandler(handler.MauiContext!).PlatformView is WSwipeItem swipeItem) + concreteHandler._isRebuildingSwipeItems = true; + } + + try + { + foreach (var item in items) { - swipeItem.BehaviorOnInvoked = items.SwipeBehaviorOnInvoked.ToPlatform(); - swipeItems.Add(swipeItem); + if (item is ISwipeItemMenuItem menuItem) + { + // Always create the handler regardless of visibility so that subsequent + // visibility changes can propagate via the handler's UpdateValue mechanism. + var itemElementHandler = item.ToHandler(handler.MauiContext!); + + if (CanAddSwipeItems(swipeItems) && + menuItem.Visibility != Visibility.Collapsed && + itemElementHandler.PlatformView is WSwipeItem swipeItem) + { + swipeItem.BehaviorOnInvoked = items.SwipeBehaviorOnInvoked.ToPlatform(); + swipeItems.Add(swipeItem); + } + } + } + } + finally + { + if (handler is SwipeViewHandler concreteHandler2) + { + concreteHandler2._isRebuildingSwipeItems = false; } }