Skip to content
8 changes: 7 additions & 1 deletion src/Controls/src/Core/SwipeView/SwipeItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/// <summary>Bindable property for <see cref="IsVisible"/>.</summary>
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);

/// <summary>
/// Gets or sets the background color of the swipe item. This is a bindable property.
Expand All @@ -40,6 +40,12 @@ public bool IsVisible

Visibility ISwipeItemMenuItem.Visibility => this.IsVisible ? Visibility.Visible : Visibility.Collapsed;

static void OnIsVisibleChanged(BindableObject bindable, object oldValue, object newValue)
{
var swipeItem = (SwipeItem)bindable;
swipeItem.Handler?.UpdateValue(nameof(ISwipeItemMenuItem.Visibility));

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[minor] Defensive castvar swipeItem = (SwipeItem)bindable; will throw if IsVisibleProperty is ever attached to a non-SwipeItem target (e.g. via styles applying to a derived type that re-uses the BP). The conventional MAUI pattern is if (bindable is SwipeItem swipeItem) swipeItem.Handler?.UpdateValue(...). Low risk because BindableProperty.Create ties the property to typeof(SwipeItem), but the safer pattern is preferred in propertyChanged callbacks throughout the codebase.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[minor] Defensive castvar swipeItem = (SwipeItem)bindable; will throw if IsVisibleProperty is ever attached to a non-SwipeItem target (e.g. via styles applying to a derived type that re-uses the BP). The conventional MAUI pattern is if (bindable is SwipeItem swipeItem) swipeItem.Handler?.UpdateValue(...). Low risk because BindableProperty.Create ties the property to typeof(SwipeItem), but the safer pattern is preferred in propertyChanged callbacks throughout the codebase.

}

void Maui.ISwipeItem.OnInvoked()
{
if (Command != null && Command.CanExecute(CommandParameter))
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
108 changes: 108 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue34832.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
using System.ComponentModel;
using System.Runtime.CompilerServices;

namespace Maui.Controls.Sample.Issues;

[Issue(IssueTracker.Github, 34832, "SwipeItem.IsVisible doesn't properly refresh native swipe items when binding value changes dynamically", PlatformAffected.Android | PlatformAffected.iOS)]
public class Issue34832 : ContentPage
{
readonly Issue34832ViewModel _viewModel = new() { IsDeleteVisible = false };
SwipeView _swipeView;

public Issue34832()
{
BindingContext = _viewModel;

SwipeItem deleteSwipeItem = new SwipeItem
{
Text = "Delete",
BackgroundColor = Colors.Green,
AutomationId = "DeleteSwipeItem"
};
deleteSwipeItem.SetBinding(SwipeItem.IsVisibleProperty, new Binding(nameof(Issue34832ViewModel.IsDeleteVisible)));

SwipeItem archiveSwipeItem = new SwipeItem
{
Text = "Archive",
BackgroundColor = Colors.Blue,
AutomationId = "ArchiveSwipeItem"
};

_swipeView = new SwipeView
{
AutomationId = "TestSwipeView",
HeightRequest = 60,
LeftItems = new SwipeItems { deleteSwipeItem, archiveSwipeItem },
Content = new Grid
{
BackgroundColor = Colors.LightGray,
Children =
{
new Label
{
Text = "Swipe left 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 openSwipeButton = new Button
{
Text = "Open Swipe",
AutomationId = "OpenSwipeButton"
};
openSwipeButton.Clicked += (s, e) => _swipeView?.Open(OpenSwipeItem.LeftItems);

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,
openSwipeButton,
resetButton,
}
};
}
}

public class Issue34832ViewModel : 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));
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#if TEST_FAILS_ON_WINDOWS // Issue Link - https://github.com/dotnet/maui/issues/35216

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[moderate] Missing Windows coverage justification — entire file is wrapped in #if TEST_FAILS_ON_WINDOWS referencing #35216. The fix in this PR only touches iOS/Android handlers (no Windows handler change), so Windows behaviour is untested. If SwipeItem.IsVisible was already broken on Windows (per #35216), please cross-link that issue from the PR description and confirm whether this PR is expected to fix Windows or leave it as-is. The current title says [iOS/Android] so leaving Windows out is intentional, but a code comment near the #if referencing the scope decision would help future reviewers.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[moderate] Missing Windows coverage justification — entire file is wrapped in #if TEST_FAILS_ON_WINDOWS referencing #35216. The fix in this PR only touches iOS/Android handlers (no Windows handler change), so Windows behaviour is untested. If SwipeItem.IsVisible was already broken on Windows (per #35216), please cross-link that issue from the PR description and confirm whether this PR is expected to fix Windows or leave it as-is. The current title says [iOS/Android] so leaving Windows out is intentional, but a code comment near the #if referencing the scope decision would help future reviewers.

using NUnit.Framework;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues;

public class Issue34832 : _IssuesUITest
{
public override string Issue => "SwipeItem.IsVisible doesn't properly refresh native swipe items when binding value changes dynamically";

public Issue34832(TestDevice device) : base(device)
{
}

[Test]
[Order(1)]
[Category(UITestCategories.SwipeView)]
public void SwipeItemInitiallyHiddenBecomesVisibleAfterBindingChanges()
{
Exception? exception = null;
App.WaitForElement("OpenSwipeButton");
App.Tap("OpenSwipeButton");

VerifyScreenshotOrSetException(ref exception, "SwipeOpen_InitiallyHidden");

App.Tap("ToggleVisibilityButton");

VerifyScreenshotOrSetException(ref exception, "SwipeOpen_BecomeVisible");

App.Tap("TestSwipeView");
App.Tap("ResetButton");

if (exception is not null)
{
throw exception;
}
}

[Test]
[Order(2)]
[Category(UITestCategories.SwipeView)]
public void SwipeItemBecomesHiddenAfterBindingChanges()
{
Exception? exception = null;
App.WaitForElement("ToggleVisibilityButton");
App.Tap("ToggleVisibilityButton");
App.Tap("OpenSwipeButton");

VerifyScreenshotOrSetException(ref exception, "SwipeOpen_DeleteVisible");

App.Tap("ToggleVisibilityButton");

VerifyScreenshotOrSetException(ref exception, "SwipeOpen_DeleteHidden");

App.Tap("TestSwipeView");
App.Tap("ResetButton");

if (exception is not null)
{
throw exception;
}
}
}
#endif
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,12 @@ public static void MapBackground(ISwipeItemMenuItemHandler handler, ISwipeItemMe

public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMenuItem view)
{
// Set visibility before UpdateIsVisibleSwipeItem so LayoutSwipeItems
// reads the correct visibility when recalculating item positions.
handler.PlatformView.Visibility = view.Visibility.ToPlatformVisibility();

var swipeView = handler.PlatformView.Parent.GetParentOfType<MauiSwipeView>();
swipeView?.UpdateIsVisibleSwipeItem(view);

handler.PlatformView.Visibility = view.Visibility.ToPlatformVisibility();
}

protected override AView CreatePlatformElement()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,11 @@ public static void MapVisibility(ISwipeItemMenuItemHandler handler, ISwipeItemMe
{
var swipeView = handler.PlatformView.GetParentOfType<MauiSwipeView>();

swipeView?.UpdateIsVisibleSwipeItem(view);

// Update the native view's Hidden state BEFORE calling UpdateIsVisibleSwipeItem,
// so LayoutSwipeItems can use the correct Hidden state when repositioning items.
handler.PlatformView.UpdateVisibility(view.Visibility);

swipeView?.UpdateIsVisibleSwipeItem(view);
}

partial class SwipeItemMenuItemImageSourcePartSetter
Expand Down
8 changes: 7 additions & 1 deletion src/Core/src/Platform/Android/MauiSwipeView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,11 @@ void LayoutSwipeItems(List<AView> childs)

foreach (var child in childs)
{
if (i >= items.Count)
{
break;
}

if (child.Visibility == ViewStates.Visible)
{
var item = items[i];
Expand Down Expand Up @@ -655,9 +660,10 @@ void LayoutSwipeItems(List<AView> childs)

child.Layout(l, t, r, b);

i++;
previousWidth += swipeItemWidth;
}

i++;
}
}

Expand Down
18 changes: 12 additions & 6 deletions src/Core/src/Platform/iOS/MauiSwipeView.cs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ void UpdateSwipeItems()
double swipeItemsWidth;

if (_swipeDirection == SwipeDirection.Left || _swipeDirection == SwipeDirection.Right)
swipeItemsWidth = items.Count * SwipeViewExtensions.SwipeItemWidth;
swipeItemsWidth = items.Count(GetIsVisible) * SwipeViewExtensions.SwipeItemWidth;
else
swipeItemsWidth = _contentView.Frame.Width;

Expand All @@ -312,6 +312,7 @@ void UpdateSwipeItems()
foreach (var item in items)
{
UIView swipeItem = item.ToPlatform(Element.Handler.MauiContext);
swipeItem.Hidden = !GetIsVisible(item);
_actionView.AddSubview(swipeItem);
_swipeItems.Add(item, swipeItem);
}
Expand Down Expand Up @@ -342,6 +343,11 @@ void LayoutSwipeItems(List<UIView> childs)

foreach (var child in childs)
{
if (i >= items.Count)
{
break;
}

if (!child.Hidden)
{
var item = items[i];
Expand Down Expand Up @@ -371,10 +377,10 @@ void LayoutSwipeItems(List<UIView> childs)
UpdateSwipeItemInsets(button);
}

i++;
previousWidth += swipeItemWidth;
}

i++;
_swipeItemsRect.Add(child.Frame);
}
}
Expand Down Expand Up @@ -626,12 +632,12 @@ void SetFrame()
{
case SwipeDirection.Left:
_contentView.Frame = new CGRect(_originalBounds.X + offset, _originalBounds.Y, _originalBounds.Width, _originalBounds.Height);
actionSize = Element.RightItems.Count * SwipeViewExtensions.SwipeItemWidth;
actionSize = Element.RightItems.Count(GetIsVisible) * SwipeViewExtensions.SwipeItemWidth;
_actionView.Frame = new CGRect(actionSize + offset, actionBounds.Y, actionBounds.Width, actionBounds.Height);
break;
case SwipeDirection.Right:
_contentView.Frame = new CGRect(_originalBounds.X + offset, _originalBounds.Y, _originalBounds.Width, _originalBounds.Height);
actionSize = Element.LeftItems.Count * SwipeViewExtensions.SwipeItemWidth;
actionSize = Element.LeftItems.Count(GetIsVisible) * SwipeViewExtensions.SwipeItemWidth;
_actionView.Frame = new CGRect(-actionSize + offset, actionBounds.Y, actionBounds.Width, actionBounds.Height);
break;
case SwipeDirection.Up:
Expand Down Expand Up @@ -841,12 +847,12 @@ void SwipeToThreshold(bool animated = true)
{
case SwipeDirection.Left:
_contentView.Frame = new CGRect(_originalBounds.X - swipeThreshold, _originalBounds.Y, _originalBounds.Width, _originalBounds.Height);
actionSize = Element.RightItems.Count * SwipeViewExtensions.SwipeItemWidth;
actionSize = Element.RightItems.Count(GetIsVisible) * SwipeViewExtensions.SwipeItemWidth;
_actionView.Frame = new CGRect(actionSize - swipeThreshold, actionBounds.Y, actionBounds.Width, actionBounds.Height);
break;
case SwipeDirection.Right:
_contentView.Frame = new CGRect(_originalBounds.X + swipeThreshold, _originalBounds.Y, _originalBounds.Width, _originalBounds.Height);
actionSize = Element.LeftItems.Count * SwipeViewExtensions.SwipeItemWidth;
actionSize = Element.LeftItems.Count(GetIsVisible) * SwipeViewExtensions.SwipeItemWidth;
_actionView.Frame = new CGRect(-actionSize + swipeThreshold, actionBounds.Y, actionBounds.Width, actionBounds.Height);
break;
case SwipeDirection.Up:
Expand Down
Loading