-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Fixed [Windows] KeepLastItemInView Behavior Not Working as Expected in CarouselView #29550
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
5e0226c
d4bd1a0
f52cbb6
e4026b5
4f07eb7
66d731e
af0a81c
fff657f
488ffeb
7ffae3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -133,7 +133,8 @@ void OnItemsVectorChanged(global::Windows.Foundation.Collections.IObservableVect | |||||||
if (sender is not ItemCollection items) | ||||||||
return; | ||||||||
|
||||||||
var itemsCount = items.Count; | ||||||||
// When looping is enabled in CarouselView, Items.Count returns the FakeCount instead of the actual item count. | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [nitpick] Consider adding an inline comment clarifying why VirtualView is being checked against CollectionView and what types are expected; this will help future maintainers understand the intended behavior when selecting between items.Count and ItemCount.
Suggested change
Copilot uses AI. Check for mistakes. Positive FeedbackNegative Feedback |
||||||||
var itemsCount = VirtualView is CollectionView ? items.Count : ItemCount; | ||||||||
|
||||||||
if (itemsCount == 0) | ||||||||
return; | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
using System.Collections.ObjectModel; | ||
|
||
namespace Maui.Controls.Sample.Issues; | ||
[Issue(IssueTracker.Github, 29420, "KeepLastInView Not Working as Expected in CarouselView", PlatformAffected.UWP)] | ||
public class Issue29420 : ContentPage | ||
{ | ||
public Issue29420() | ||
{ | ||
var count = 0; | ||
var verticalStackLayout = new VerticalStackLayout(); | ||
var carouselItems = new ObservableCollection<string> | ||
{ | ||
"Item 0", "Item 1","Item 2", "Item 3", "Item 4", "Item 5", | ||
|
||
}; | ||
|
||
CarouselView carouselView = new CarouselView | ||
{ | ||
ItemsSource = carouselItems, | ||
AutomationId = "CarouselView", | ||
Loop = true, | ||
ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepLastItemInView, | ||
|
||
ItemTemplate = new DataTemplate(() => | ||
{ | ||
var grid = new Grid | ||
{ | ||
Padding = 10 | ||
}; | ||
|
||
var label = new Label | ||
{ | ||
VerticalOptions = LayoutOptions.Center, | ||
HorizontalOptions = LayoutOptions.Center, | ||
FontSize = 18, | ||
Background = Colors.Pink, | ||
}; | ||
label.SetBinding(Label.TextProperty, "."); | ||
label.SetBinding(Label.AutomationIdProperty, "."); | ||
|
||
grid.Children.Add(label); | ||
return grid; | ||
}), | ||
}; | ||
|
||
var insertButton = new Button | ||
{ | ||
Text = "Insert item at 0th index", | ||
AutomationId = "InsertButton", | ||
Margin = new Thickness(20), | ||
}; | ||
|
||
var addButton = new Button | ||
{ | ||
Text = "Add item at end", | ||
AutomationId = "AddButton", | ||
Margin = new Thickness(20), | ||
}; | ||
|
||
insertButton.Clicked += (sender, e) => | ||
{ | ||
carouselItems.Insert(0, "NewItem" + count.ToString()); | ||
count++; | ||
}; | ||
|
||
addButton.Clicked += (sender, e) => | ||
{ | ||
carouselItems.Add("NewItem"); | ||
}; | ||
|
||
verticalStackLayout.Children.Add(insertButton); | ||
verticalStackLayout.Children.Add(addButton); | ||
verticalStackLayout.Children.Add(carouselView); | ||
Content = verticalStackLayout; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
#if TEST_FAILS_ON_IOS && TEST_FAILS_ON_CATALYST // CarouselView Fails to Keep Last Item in View on iOS and macOS https://github.com/dotnet/maui/issues/18029 | ||
using NUnit.Framework; | ||
using UITest.Appium; | ||
using UITest.Core; | ||
|
||
namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
public class Issue29420 : _IssuesUITest | ||
{ | ||
public override string Issue => "KeepLastInView Not Working as Expected in CarouselView"; | ||
|
||
public Issue29420(TestDevice device) : base(device) | ||
{ } | ||
|
||
[Test] | ||
[Category(UITestCategories.CarouselView)] | ||
public void VerifyCarouselViewKeepLastInViewOnItemAdd() | ||
{ | ||
App.WaitForElement("CarouselView"); | ||
for (var i = 0; i < 5; i++) | ||
{ | ||
App.Tap("InsertButton"); | ||
} | ||
|
||
App.WaitForElement("Item 5"); | ||
App.Tap("AddButton"); | ||
App.WaitForElement("NewItem"); | ||
} | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] It would be helpful to add a brief comment explaining the rationale for including the check against ItemsView.Position, so that the intention to prevent unnecessary scrolling when the current item is already visible is clearly documented.
Copilot uses AI. Check for mistakes.