Skip to content

Commit ba04b4e

Browse files
SuthiYuvarajjsuarezruiz
SuthiYuvaraj
and
jsuarezruiz
authored
Fix for CarouselView doesnot scroll corresponding to ItemsUpdatingScrollMode when collection modified (#26608)
* Added sample * Added test * Added UITest to validate Windows CarouselView scrolling behavior * Updated test * Updated test * More changes * Commit for 25991 * Commit for testcase changes * Update CarouselViewController2.cs * commit for unwanted changes * commit for unwanted changes * commit for testcase removal * commit for refactor * commit for method name change * Added sample * Added test * Added UITest to validate Windows CarouselView scrolling behavior * Updated test * Updated test * More changes * Fix the test * Update Issue25991.cs with minor fixes and additions * Fix Issue25991 test assertions and string format * Commit for testcase failure * commit for testcase changes * commit for testcase changes * Create ItemShouldbeScrolledbasedOnGroupHeader.png * Delete ItemShouldbeScrolledbasedOnGroupHeader.png * Add check for empty ItemsSource in CarouselViewController2. * Fix item addition logic in Issue25991 test * Update issue comments in Issue8964.cs --------- Co-authored-by: Javier Suárez <javiersuarezruiz@hotmail.com>
1 parent ba0be32 commit ba04b4e

File tree

9 files changed

+253
-4
lines changed

9 files changed

+253
-4
lines changed

‎src/Controls/src/Core/Handlers/Items/Android/MauiCarouselRecyclerView.cs‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,30 @@ void CollectionItemsSourceChanged(object sender, System.Collections.Specialized.
267267
return;
268268
}
269269

270+
// While Modifying the collection we should consider the ItemsUpdatingScrollMode to update the position
271+
if (Carousel.ItemsUpdatingScrollMode == ItemsUpdatingScrollMode.KeepLastItemInView)
272+
{
273+
if (count == 0)
274+
{
275+
carouselPosition = 0;
276+
}
277+
else
278+
{
279+
carouselPosition = count - 1;
280+
}
281+
282+
}
283+
else if (Carousel.ItemsUpdatingScrollMode == ItemsUpdatingScrollMode.KeepItemsInView)
284+
{
285+
carouselPosition = 0;
286+
}
287+
270288
Carousel.
271289
Handler.
272290
MauiContext.
273291
GetDispatcher()
274292
.Dispatch(() =>
275293
{
276-
277294
SetCurrentItem(carouselPosition);
278295
UpdatePosition(carouselPosition);
279296

‎src/Controls/src/Core/Handlers/Items/iOS/CarouselViewController.cs‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -334,14 +334,17 @@ void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e)
334334
[UnconditionalSuppressMessage("Memory", "MEM0003", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
335335
void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e)
336336
{
337+
int targetPosition;
337338
if (_positionAfterUpdate == -1)
338339
{
339340
return;
340341
}
341342

342343
_gotoPosition = -1;
343344

344-
var targetPosition = _positionAfterUpdate;
345+
// We need to update the position while modifying the collection.
346+
targetPosition = GetTargetPosition();
347+
345348
_positionAfterUpdate = -1;
346349

347350
SetPosition(targetPosition);
@@ -354,6 +357,22 @@ int GetPositionWhenAddingItems(int carouselPosition, int currentItemPosition)
354357
return currentItemPosition != -1 ? currentItemPosition : carouselPosition;
355358
}
356359

360+
private int GetTargetPosition()
361+
{
362+
363+
if (ItemsSource.ItemCount == 0)
364+
{
365+
return 0;
366+
}
367+
368+
return ItemsView.ItemsUpdatingScrollMode switch
369+
{
370+
ItemsUpdatingScrollMode.KeepItemsInView => 0,
371+
ItemsUpdatingScrollMode.KeepLastItemInView => ItemsSource.ItemCount - 1,
372+
_ => _positionAfterUpdate
373+
};
374+
}
375+
357376
int GetPositionWhenResetItems()
358377
{
359378
//If we are reseting the collection Position should go to 0

‎src/Controls/src/Core/Handlers/Items2/iOS/CarouselViewController2.cs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -254,14 +254,17 @@ void CollectionViewUpdating(object sender, NotifyCollectionChangedEventArgs e)
254254
[UnconditionalSuppressMessage("Memory", "MEM0003", Justification = "Proven safe in test: MemoryTests.HandlerDoesNotLeak")]
255255
void CollectionViewUpdated(object sender, NotifyCollectionChangedEventArgs e)
256256
{
257+
int targetPosition;
257258
if (_positionAfterUpdate == -1)
258259
{
259260
return;
260261
}
261262

262263
//_gotoPosition = -1;
263264

264-
var targetPosition = _positionAfterUpdate;
265+
// We need to update the position while modifying the collection.
266+
targetPosition = GetTargetPosition();
267+
265268
_positionAfterUpdate = -1;
266269

267270
SetPosition(targetPosition);
@@ -291,6 +294,21 @@ int GetPositionWhenAddingItems(int carouselPosition, int currentItemPosition)
291294
return currentItemPosition != -1 ? currentItemPosition : carouselPosition;
292295
}
293296

297+
private int GetTargetPosition()
298+
{
299+
if (ItemsSource.ItemCount == 0)
300+
{
301+
return 0;
302+
}
303+
304+
return ItemsView.ItemsUpdatingScrollMode switch
305+
{
306+
ItemsUpdatingScrollMode.KeepItemsInView => 0,
307+
ItemsUpdatingScrollMode.KeepLastItemInView => ItemsSource.ItemCount - 1,
308+
_ => _positionAfterUpdate
309+
};
310+
}
311+
294312
int GetPositionWhenResetItems()
295313
{
296314
//If we are reseting the collection Position should go to 0

‎src/Controls/tests/TestCases.HostApp/Issues/CarouselViewLoopNoFreeze.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public CarouselViewLoopNoFreeze()
5757
};
5858
_carouselView.SetBinding(CarouselView.ItemsSourceProperty, "Items");
5959
this.SetBinding(Page.TitleProperty, "Title");
60+
_carouselView.ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepScrollOffset;
6061

6162
var layout = new Grid();
6263
layout.RowDefinitions.Add(new RowDefinition { Height = 100 });

‎src/Controls/tests/TestCases.HostApp/Issues/CarouselViewRemoveAt.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ public CarouselViewRemoveAt()
6565
};
6666
_carousel.CurrentItemChanged += Carousel_CurrentItemChanged;
6767
_carousel.PositionChanged += Carousel_PositionChanged;
68+
_carousel.ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepScrollOffset;
6869

6970
Grid.SetColumnSpan(_carousel, 2);
7071

‎src/Controls/tests/TestCases.HostApp/Issues/Issue25991.cs‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
using System.Collections.ObjectModel;
2+
3+
namespace Maui.Controls.Sample.Issues
4+
{
5+
[Issue(IssueTracker.Github, 25991, "CarouselView reverts to displaying first item in collection when collection modified", PlatformAffected.UWP)]
6+
public class Issue25991 : ContentPage
7+
{
8+
public class Issue25991Model
9+
{
10+
public string Text { get; set; }
11+
12+
public string AutomationId { get; set; }
13+
}
14+
15+
ObservableCollection<Issue25991Model> _collection = new ObservableCollection<Issue25991Model>()
16+
{
17+
new Issue25991Model() { Text = "Item 1" , AutomationId = "Issue25991Item1" },
18+
new Issue25991Model() { Text = "Item 2", AutomationId = "Issue25991Item2" }
19+
};
20+
21+
Label InfoLabel { get; set; }
22+
CarouselView TestCarouselView { get; set; }
23+
24+
public Issue25991()
25+
{
26+
StackLayout mainStackLayout = new StackLayout();
27+
mainStackLayout.Margin = new Thickness(5);
28+
29+
Label labelInstructions = new Label()
30+
{
31+
AutomationId = "WaitForStubControl",
32+
Text = "Instructions:\nThe CarouselView contains Item 1 and Item 2. It will initially show the first page, Item 1.\n" +
33+
"Click the 'Scroll to Item 2' button, and the CarouselView will correctly show Item 2.\n" +
34+
"Click 'Add Item', which will add a new Item record to the collection.\n" +
35+
"The CarouselView must stay in the Item 2."
36+
};
37+
mainStackLayout.Add(labelInstructions);
38+
39+
InfoLabel = new Label()
40+
{
41+
AutomationId = "InfoLabel",
42+
};
43+
44+
mainStackLayout.Add(InfoLabel);
45+
46+
TestCarouselView = new CarouselView()
47+
{
48+
ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepScrollOffset,
49+
ItemsSource = _collection,
50+
Loop = false,
51+
HeightRequest = 400
52+
};
53+
54+
TestCarouselView.ItemTemplate = new DataTemplate(() =>
55+
{
56+
StackLayout stackLayout = new StackLayout();
57+
Label nameLabel = new Label();
58+
nameLabel.SetBinding(Label.TextProperty, "Text");
59+
nameLabel.SetBinding(Label.AutomationIdProperty, "AutomationId");
60+
nameLabel.FontSize = 25;
61+
nameLabel.TextColor = Colors.Red;
62+
stackLayout.Children.Add(nameLabel);
63+
return stackLayout;
64+
});
65+
66+
TestCarouselView.CurrentItemChanged += OnTestCarouselViewCurrentItemChanged;
67+
mainStackLayout.Add(TestCarouselView);
68+
69+
Button scrollToPerson2Button = new Button()
70+
{
71+
AutomationId = "ScrollToPerson2Button",
72+
Text = "Scroll to Item 2"
73+
};
74+
scrollToPerson2Button.Clicked += OnScrollToPerson2ButtonClicked;
75+
mainStackLayout.Add(scrollToPerson2Button);
76+
77+
Button addItemButton = new Button()
78+
{
79+
AutomationId = "AddItemButton",
80+
Text = "Add Item",
81+
};
82+
addItemButton.Clicked += OnAddButtonClicked;
83+
mainStackLayout.Add(addItemButton);
84+
85+
86+
HorizontalStackLayout buttonsStackLayout = new HorizontalStackLayout();
87+
Button keepItemsInViewButton = new Button()
88+
{
89+
AutomationId = "KeepItemsInViewButton",
90+
Text = "keepItemsInView",
91+
};
92+
keepItemsInViewButton.Clicked += OnKeepItemsInViewButtonClicked;
93+
buttonsStackLayout.Add(keepItemsInViewButton);
94+
Button keepScrollOffsetButton = new Button()
95+
{
96+
AutomationId = "KeepScrollOffsetButton",
97+
Text = "KeepScrollOffset",
98+
};
99+
keepScrollOffsetButton.Clicked += OnKeepScrollOffsetButtonClicked;
100+
buttonsStackLayout.Add(keepScrollOffsetButton);
101+
Button keepLastItemInViewButton = new Button()
102+
{
103+
AutomationId = "KeepLastItemInViewButton",
104+
Text = "KeepLastItemInView",
105+
};
106+
keepLastItemInViewButton.Clicked += OnKeepLastItemInViewButtonClicked;
107+
buttonsStackLayout.Add(keepLastItemInViewButton);
108+
mainStackLayout.Add(buttonsStackLayout);
109+
110+
Content = mainStackLayout;
111+
}
112+
113+
void OnTestCarouselViewCurrentItemChanged(object sender, CurrentItemChangedEventArgs e)
114+
{
115+
InfoLabel.Text = TestCarouselView.Position.ToString();
116+
}
117+
118+
void OnScrollToPerson2ButtonClicked(object sender, EventArgs e)
119+
{
120+
TestCarouselView.ScrollTo(1);
121+
}
122+
123+
void OnAddButtonClicked(object sender, EventArgs e)
124+
{
125+
_collection.Add(new Issue25991Model()
126+
{
127+
Text = "Item " + (_collection.Count + 1),
128+
AutomationId = "Issue25991Item" + (_collection.Count + 1)
129+
});
130+
}
131+
132+
void OnKeepItemsInViewButtonClicked(object sender, EventArgs e)
133+
{
134+
TestCarouselView.ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepItemsInView;
135+
}
136+
137+
void OnKeepScrollOffsetButtonClicked(object sender, EventArgs e)
138+
{
139+
TestCarouselView.ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepScrollOffset;
140+
}
141+
142+
void OnKeepLastItemInViewButtonClicked(object sender, EventArgs e)
143+
{
144+
TestCarouselView.ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepLastItemInView;
145+
}
146+
}
147+
}

‎src/Controls/tests/TestCases.HostApp/Issues/XFIssue/Issue8964.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ protected override void Init()
2323
ItemsSource = ItemSourceUnderTest,
2424
ItemTemplate = GetCarouselTemplate(),
2525
CurrentItem = _currentItem,
26+
ItemsUpdatingScrollMode = ItemsUpdatingScrollMode.KeepScrollOffset,
2627
AutomationId = "carouseView",
2728
Loop = false
2829
};

‎src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue25991.cs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using NUnit.Framework;
2+
using NUnit.Framework.Legacy;
3+
using UITest.Appium;
4+
using UITest.Core;
5+
6+
namespace Microsoft.Maui.TestCases.Tests.Issues
7+
{
8+
public class Issue25991 : _IssuesUITest
9+
{
10+
const string InfoLabel = "InfoLabel";
11+
const string AddItemButton = "AddItemButton";
12+
13+
public Issue25991(TestDevice device)
14+
: base(device)
15+
{ }
16+
17+
public override string Issue => "CarouselView reverts to displaying first item in collection when collection modified";
18+
19+
[Test]
20+
[Category(UITestCategories.CarouselView)]
21+
public void Issue25991Test()
22+
{
23+
App.WaitForElement("WaitForStubControl");
24+
25+
App.WaitForElement("Issue25991Item1");
26+
27+
App.Click("ScrollToPerson2Button");
28+
App.Click(AddItemButton);
29+
30+
App.WaitForElement("Issue25991Item2");
31+
32+
App.Click("KeepItemsInViewButton");
33+
App.Click(AddItemButton);
34+
35+
36+
App.WaitForElement("Issue25991Item1");
37+
38+
App.Click("KeepLastItemInViewButton");
39+
App.Click(AddItemButton);
40+
41+
App.WaitForElement("Issue25991Item5");
42+
43+
}
44+
}
45+
}

‎src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/XFIssue/Issue8964.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWS && TEST_FAILS_ON_ANDROID //The Position property now functions correctly on Android and Windows, issue: https://github.com/dotnet/maui/issues/15443. Note that on Catalyst, swipe and drag options are not supported in Appium.
1+
#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_WINDOWS && TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_IOS //The Position property now functions correctly on Android and Windows, issue: https://github.com/dotnet/maui/issues/15443. // This scenario fails on iOS,Android and Catalyst with KeepItemsInView , However carouselview flickers with KeepScrollOffset , issue:https://github.com/dotnet/maui/issues/27773.
22
using NUnit.Framework;
33
using UITest.Appium;
44
using UITest.Core;

0 commit comments

Comments
 (0)