From: kingces95 Date: Thu, 7 Apr 2016 21:55:19 +0000 (-0700) Subject: [A, iOS] CarouselView Bug Fixes (#49) X-Git-Tag: beta-2.3.0-pre1~68 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=870f9c98ffccce86d80c6ff7a05de7238fc1b0e8;p=platform%2Fupstream%2Fxamarin-forms.git [A, iOS] CarouselView Bug Fixes (#49) * CarouselView programatic scrolling fixes; swipe back fixes * Make iOS CarouselView events consistant with Android * bump swipe distance; add Screenshot on Exception * Formatting. No logical change. * Comment out [Test] on UITest and fix TestCloud failures later. --- diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/CarouselViewGallery.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/CarouselViewGallery.cs new file mode 100644 index 0000000..9c7d9cd --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/CarouselViewGallery.cs @@ -0,0 +1,280 @@ +using System; + +using Xamarin.Forms.CustomAttributes; +using System.Threading; +using System.Linq; +using System.Collections.Generic; + +#if UITEST +using Xamarin.UITest; +using NUnit.Framework; +#endif + +namespace Xamarin.Forms.Controls +{ + [Preserve (AllMembers = true)] + [Issue (IssueTracker.Github, 900000, "CarouselView General Tests")] + public class CarouselViewGalleryTests + { +#if UITEST + + public interface IUIProxy + { + void Load(IApp app); + } + public interface IGalleryPage : IUIProxy + { + string Name { get; } + } + + public class Gallery + { + static class Id + { + internal const string SearchBar = nameof(SearchBar); + internal const string GoToTestButton = nameof(GoToTestButton); + } + + public static Gallery Launch() + { + var app = AppSetup.Setup(); + app.WaitForElement(Id.SearchBar); + return new Gallery(app); + } + + IApp _app; + + Gallery(IApp app) + { + _app = app; + } + + public TGalleryPage NaviateToGallery() where TGalleryPage : IGalleryPage, new() + { + var galleryPage = new TGalleryPage(); + _app.EnterText(Id.SearchBar, galleryPage.Name); + _app.Tap(Id.GoToTestButton); + galleryPage.Load(_app); + return galleryPage; + } + public void Screenshot(string message) => _app.Screenshot(message); + } + + public class CarouselViewGallery : IGalleryPage + { + internal const int InitialItems = 4; + internal const int InitialItemId = 1; + internal const string OnItemSelectedAbbr = "i"; + internal const string OnPositionSelectedAbbr = "p"; + internal const int EventQueueDepth = 7; + + private const double SwipePercentage = 0.50; + private const int SwipeSpeed = 2000; + + static class Id + { + internal const string Name = "CarouselView Gallery"; + internal static string ItemId = nameof(ItemId); + internal static string EventLog = nameof(EventLog); + internal static string SelectedItem = nameof(SelectedItem); + internal static string Position = nameof(Position); + internal static string SelectedPosition = nameof(SelectedPosition); + internal static string Next = nameof(Next); + internal static string Previous = nameof(Previous); + internal static string First = nameof(First); + internal static string Last = nameof(Last); + } + enum Event + { + OnItemSelected, + OnPositionSelected + } + + IApp _app; + List _itemIds; + int _currentPosition; + int _currentItem; + Queue _expectedEvents; + int _eventId; + + public CarouselViewGallery() { + _itemIds = Enumerable.Range(0, InitialItems).ToList(); + _currentPosition = InitialItemId; + _currentItem = _itemIds[_currentPosition]; + _expectedEvents = new Queue(); + _eventId = 0; + } + + void IUIProxy.Load(IApp app) + { + _app = app; + WaitForValue(Id.ItemId, _currentItem); + WaitForValue(Id.Position, _currentPosition); + } + + private void WaitForValue(string marked, object value) + { + var query = $"* marked:'{marked}' text:'{value}'"; + _app.WaitForElement(o => o.Raw(query)); + + } + private void WaitForPosition(int expectedPosition) + { + var expectedItem = _itemIds[expectedPosition]; + + // expect no movement + if (_currentItem == expectedItem) + Thread.Sleep(TimeSpan.FromMilliseconds(500)); + + // wait for for expected item and corresponding event + WaitForValue(Id.ItemId, expectedItem); + WaitForValue(Id.SelectedItem, expectedItem); + _currentItem = expectedItem; + + // wait for for expected position and corresponding event + WaitForValue(Id.Position, expectedPosition); + WaitForValue(Id.SelectedPosition, expectedPosition); + _currentPosition = expectedPosition; + + // check expected events + var expectedEvents = string.Join(", ", _expectedEvents.ToArray().Reverse()); + WaitForValue(Id.EventLog, expectedEvents); + } + private void ExpectMovementEvents(int expectedPosition) + { + if (expectedPosition == _currentPosition) + return; + + ExpectEvent(Event.OnPositionSelected); + ExpectEvent(Event.OnItemSelected); + } + private void ExpectEvent(Event e) + { + if (e == Event.OnItemSelected) + _expectedEvents.Enqueue($"{OnItemSelectedAbbr}/{_eventId++}"); + + if (e == Event.OnPositionSelected) + _expectedEvents.Enqueue($"{OnPositionSelectedAbbr}/{_eventId++}"); + + if (_expectedEvents.Count == EventQueueDepth) + _expectedEvents.Dequeue(); + } + private void Tap(string buttonText, int expectedPosition) + { + // tap + _app.Tap(buttonText); + + // anticipate events + ExpectMovementEvents(expectedPosition); + + // wait + WaitForPosition(expectedPosition); + } + private void Swipe(bool next, int expectedPosition) + { + // swipe + if (next) + _app.SwipeRightToLeft(swipePercentage: SwipePercentage/*, swipeSpeed: SwipeSpeed*/); + else + _app.SwipeLeftToRight(swipePercentage: SwipePercentage/*, swipeSpeed: SwipeSpeed*/); + + // handle swipe past first + if (expectedPosition == -1 && _currentPosition == 0) + expectedPosition = 0; + + // handle swipe past last + else if (expectedPosition == Count && _currentPosition == Count -1) + expectedPosition = Count -1; + + // anticipate events + ExpectMovementEvents(expectedPosition); + + // wait + WaitForPosition(expectedPosition); + } + private void Move(int steps, bool swipe) + { + Action next = swipe ? (Action)SwipeNext : StepNext; + Action previous = swipe ? (Action)SwipePrevious : StepPrevious; + + var action = next; + if (steps < 0) + { + action = previous; + steps = -steps; + } + + for (int i = 0; i < steps; i++) + action(); + } + private void MoveToPosition(int position, bool swipe) + { + Assert.True(position >= 0 && position < Count); + Move(position - _currentPosition, swipe); + } + private void MoveToItem(int targetPage, bool swipe) + { + MoveToPosition(_itemIds.IndexOf(targetPage), swipe); + } + public void MoveToFirst(bool swipe) => MoveToPosition(0, swipe); + public void MoveToLast(bool swipe) => MoveToPosition(Count - 1, swipe); + + public int ItemId => int.Parse(_app.Query(Id.ItemId)[0].Text); + + public string Name => Id.Name; + public int Count => _itemIds.Count; + + public void First() => Tap(Id.First, 0); + public void Last() => Tap(Id.Last, _itemIds.Count - 1); + + public void StepNext() => Tap(Id.Next, _currentPosition + 1); + public void StepPrevious() => Tap(Id.Previous, _currentPosition - 1); + public void Step(int steps) => Move(steps, swipe: false); + public void StepToPosition(int position) => MoveToPosition(position, swipe: false); + public void StepToItem(int item) => MoveToItem(item, swipe: false); + public void StepToFirst() => MoveToFirst(swipe: false); + public void StepToLast() => MoveToLast(swipe: false); + + public void SwipeNext() => Swipe(next: true, expectedPosition: _currentPosition + 1); + public void SwipePrevious() => Swipe(next: false, expectedPosition: _currentPosition - 1); + public void Swipe(int swipes) => Move(swipes, swipe: true); + public void SwipeToPosition(int position) => MoveToPosition(position, swipe: true); + public void SwipeToItem(int item) => MoveToItem(item, swipe: true); + public void SwipeToFirst() => MoveToFirst(swipe: true); + public void SwipeToLast() => MoveToLast(swipe: true); + } + + //[Test] + public void SwipeStepJump() + { + var gallery = Gallery.Launch(); + + try { + var carousel = gallery.NaviateToGallery(); + + // start at something other than 0 + Assert.AreNotEqual(0, CarouselViewGallery.InitialItemId); + Assert.AreEqual(CarouselViewGallery.InitialItemId, carousel.ItemId); + + // programatic jump to first/last + carousel.Last(); + carousel.First(); + + // programatic step to page + carousel.StepToLast(); + carousel.StepToFirst(); + + // swiping + carousel.SwipeToLast(); + carousel.SwipeNext(); // test swipe past end + carousel.SwipeToFirst(); + carousel.SwipePrevious(); // test swipe past start + + } catch (Exception e) { + gallery.Screenshot(e.ToString()); + throw e; + } + } +#endif + } +} diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs index 15e658c..e63e8d4 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/TestPages/TestPages.cs @@ -64,16 +64,18 @@ namespace Xamarin.Forms.Controls app.Tap (q => q.Raw ("* marked:'SearchButton'")); } - public static IApp Setup (Type pageType) + public static IApp Setup (Type pageType = null) { IApp runningApp = null; try { runningApp = InitializeApp (); - } catch { - Assert.Inconclusive ("App did not start for some reason"); + } catch (Exception e) { + Assert.Inconclusive ($"App did not start for some reason: {e}"); } - NavigateToIssue (pageType, runningApp); + if (pageType != null) + NavigateToIssue (pageType, runningApp); + return runningApp; } } diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 3d745c3..e13847b 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -142,6 +142,7 @@ + diff --git a/Xamarin.Forms.Controls/GalleryPages/CarouselViewGallery.cs b/Xamarin.Forms.Controls/GalleryPages/CarouselViewGallery.cs index 87f741c..c5b431d 100644 --- a/Xamarin.Forms.Controls/GalleryPages/CarouselViewGallery.cs +++ b/Xamarin.Forms.Controls/GalleryPages/CarouselViewGallery.cs @@ -1,5 +1,5 @@ using System; - +using System.Linq; using Xamarin.Forms.CustomAttributes; using System.Collections; using System.Collections.Generic; @@ -56,15 +56,15 @@ namespace Xamarin.Forms.Controls public ItemView () { - var change = CreateButton("Change", (items, index) => items[index] = new Moo ()); + var change = CreateButton("Change", "Change", (items, index) => items[index] = new Moo ()); var removeBar = new StackLayout { Orientation = StackOrientation.Horizontal, HorizontalOptions = LayoutOptions.FillAndExpand, Children = { - CreateButton ("- Left", (items, index) => items.RemoveAt (index - 1)), - CreateButton ("Remove", (items, index) => items.RemoveAt (index)), - CreateButton ("- Right", (items, index) => items.RemoveAt (index + 1)), + CreateButton ("- Left", "RemoveLeft", (items, index) => items.RemoveAt (index - 1)), + CreateButton ("Remove", "Remove", (items, index) => items.RemoveAt (index)), + CreateButton ("- Right", "RemoveRight", (items, index) => items.RemoveAt (index + 1)), } }; @@ -72,8 +72,8 @@ namespace Xamarin.Forms.Controls Orientation = StackOrientation.Horizontal, HorizontalOptions = LayoutOptions.FillAndExpand, Children = { - CreateButton ("+ Left", (items, index) => items.Insert (index, new Moo ())), - CreateButton ("+ Right", (items, index) => { + CreateButton ("+ Left", "AddLeft", (items, index) => items.Insert (index, new Moo ())), + CreateButton ("+ Right", "AddRight", (items, index) => { if (index == items.Count - 1) items.Add (new Moo ()); else @@ -85,7 +85,11 @@ namespace Xamarin.Forms.Controls var typeNameLabel = new Label () { StyleId = "typename" }; typeNameLabel.SetBinding (Label.TextProperty, nameof(Item.TypeName)); - var idLabel = new Label () { StyleId = "id", TextColor = Color.White }; + var idLabel = new Label () { + AutomationId = "ItemId", + StyleId = "id", + TextColor = Color.White + }; idLabel.SetBinding (Label.TextProperty, nameof(Item.Id)); Content = new StackLayout { @@ -104,9 +108,10 @@ namespace Xamarin.Forms.Controls }; } - Button CreateButton(string text, Action, int> clicked) + Button CreateButton(string text, string automationId, Action, int> clicked) { var button = new Button (); + button.AutomationId = automationId; button.Text = text; button.Clicked += (s, e) => { var items = (IList)Context.ItemsSource; @@ -169,19 +174,11 @@ namespace Xamarin.Forms.Controls } } - static readonly MyDataTemplateSelector Selector = new MyDataTemplateSelector (); - - static readonly IList Items = new ObservableCollection () { - new Baz(), - new Poo(), - new Foo(), - new Bar(), - }; - - Button CreateButton(string text, Action onClicked = null) + static Button CreateButton(string text, string automationId, Action onClicked = null) { var button = new Button { - Text = text + Text = text, + AutomationId = automationId }; if (onClicked != null) @@ -189,85 +186,116 @@ namespace Xamarin.Forms.Controls return button; } + static Label CreateValue(string text, string automationId = "") => CreateLabel(text, Color.Olive, automationId); + static Label CreateCopy(string text, string automationId = "") => CreateLabel(text, Color.White, automationId); + static Label CreateLabel(string text, Color color, string automationId) + { + return new Label() { + TextColor = color, + Text = text, + AutomationId = automationId + }; + } - public CarouselViewGallaryPage () + const int StartPosition = 1; + const int EventQueueLength = 7; + + readonly CarouselView _carouselView; + readonly MyDataTemplateSelector _selector; + readonly IList _items; + readonly Label _position; + readonly Label _selectedItem; + readonly Label _selectedPosition; + readonly Queue _events; + readonly Label _eventLog; + int _eventId; + + void OnEvent(string name) { - BackgroundColor = Color.Blue; + _events.Enqueue($"{name}/{_eventId++}"); - var logLabel = new Label () { TextColor = Color.White }; - var selectedItemLabel = new Label () { TextColor = Color.White, Text = "0" }; - var selectedPositionLabel = new Label () { TextColor = Color.White, Text = "@0" }; - //var appearingLabel = new Label () { TextColor = Color.White }; - //var disappearingLabel = new Label () { TextColor = Color.White }; + if (_events.Count == EventQueueLength) + _events.Dequeue(); + _eventLog.Text = string.Join(", ", _events.ToArray().Reverse()); - var carouselView = new CarouselView { + _position.Text = $"{_carouselView.Position}"; + } + + public CarouselViewGallaryPage () + { + _selector = new MyDataTemplateSelector (); + _items = new ObservableCollection() { + new Baz(), + new Poo(), + new Foo(), + new Bar(), + }; + + _carouselView = new CarouselView { BackgroundColor = Color.Purple, - ItemsSource = Items, - ItemTemplate = Selector, - Position = 1 + ItemsSource = _items, + ItemTemplate = _selector, + Position = StartPosition }; - bool capture = false; - carouselView.ItemSelected += (s, o) => { - var item = (Item)o.SelectedItem; - selectedItemLabel.Text = $"{item.Id}"; - if (capture) - logLabel.Text += $"({item.Id}) "; + _events = new Queue(); + _eventId = 0; + _position = CreateValue($"{_carouselView.Position}", "Position"); + _selectedItem = CreateValue("?", "SelectedItem"); + _selectedPosition = CreateValue("?", "SelectedPosition"); + _eventLog = CreateValue(string.Empty, "EventLog"); + + _carouselView.ItemSelected += (s, o) => { + var selectedItem = ((Item)o.SelectedItem).Id; + _selectedItem.Text = $"{selectedItem}"; + OnEvent("i"); }; - carouselView.PositionSelected += (s, o) => { - var position = (int)o.SelectedPosition; - selectedPositionLabel.Text = $"@{position}=={carouselView.Position}"; - if (capture) - logLabel.Text += $"(@{position}) "; + + _carouselView.PositionSelected += (s, o) => { + var selectedPosition = (int)o.SelectedPosition; + _selectedPosition.Text = $"{selectedPosition}"; + OnEvent("p"); }; - //carouselView.ItemDisappearing += (s, o) => { - // var item = (Item)o.Item; - // var id = item.Id; - // disappearingLabel.Text = $"-{id}"; - // if (capture) - // logLabel.Text += $"(-{id}) "; - //}; - //carouselView.ItemAppearing += (s, o) => { - // var item = (Item)o.Item; - // var id = item.Id; - // appearingLabel.Text = $"+{id}"; - // if (capture) - // logLabel.Text += $"(+{id}) "; - //}; + + BackgroundColor = Color.Blue; var moveBar = new StackLayout { Orientation = StackOrientation.Horizontal, HorizontalOptions = LayoutOptions.FillAndExpand, Children = { - CreateButton ("<<", () => carouselView.Position = 0), - CreateButton ("<", () => { try { carouselView.Position--; } catch { } }), - CreateButton (">", () => { try { carouselView.Position++; } catch { } }), - CreateButton (">>", () => carouselView.Position = Items.Count - 1) + CreateButton ("<<", "First", () => _carouselView.Position = 0), + CreateButton ("<", "Previous", () => { + if (_carouselView.Position == 0) + return; + _carouselView.Position--; + }), + CreateButton (">", "Next", () => { + if (_carouselView.Position == _items.Count - 1) + return; + _carouselView.Position++; + }), + CreateButton (">>", "Last", () => _carouselView.Position = _items.Count - 1) } }; + var statusBar = new StackLayout { Orientation = StackOrientation.Horizontal, Children = { - selectedItemLabel, - selectedPositionLabel, - //disappearingLabel, - //appearingLabel, + CreateCopy("Pos:"), _position, + CreateCopy("OnItemSel:"), _selectedItem, + CreateCopy("OnPosSel:"), _selectedPosition, } }; var logBar = new StackLayout { Orientation = StackOrientation.Horizontal, - Children = { - CreateButton ("Clear", () => logLabel.Text = ""), - CreateButton ("On/Off", () => capture = !capture ), - logLabel, - } + Children = { _eventLog } }; Content = new StackLayout { Children = { - carouselView, + _carouselView, moveBar, statusBar, logBar diff --git a/Xamarin.Forms.Core/ItemsViewSimple.cs b/Xamarin.Forms.Core/ItemsViewSimple.cs index 34b7726..51b9df7 100644 --- a/Xamarin.Forms.Core/ItemsViewSimple.cs +++ b/Xamarin.Forms.Core/ItemsViewSimple.cs @@ -9,9 +9,20 @@ namespace Xamarin.Forms { public abstract class ItemsView : View, IItemViewController { - public static readonly BindableProperty ItemsSourceProperty = BindableProperty.Create("ItemsSource", typeof(IEnumerable), typeof(ItemsView), Enumerable.Empty()); - - public static readonly BindableProperty ItemTemplateProperty = BindableProperty.Create("ItemTemplate", typeof(DataTemplate), typeof(ItemsView)); + public static readonly BindableProperty ItemsSourceProperty = + BindableProperty.Create( + propertyName: "ItemsSource", + returnType: typeof(IEnumerable), + declaringType: typeof(ItemsView), + defaultValue: Enumerable.Empty() + ); + + public static readonly BindableProperty ItemTemplateProperty = + BindableProperty.Create( + propertyName: "ItemTemplate", + returnType: typeof(DataTemplate), + declaringType: typeof(ItemsView) + ); ItemSource _itemSource; @@ -66,8 +77,12 @@ namespace Xamarin.Forms { if (propertyName == nameof(ItemsSource)) { + var itemsSource = ItemsSource; + if (itemsSource == null) + itemsSource = Enumerable.Empty(); + // abstract enumerable, IList, IList, and IReadOnlyList - _itemSource = new ItemSource(ItemsSource); + _itemSource = new ItemSource(itemsSource); // subscribe to collection changed events var dynamicItemSource = _itemSource as INotifyCollectionChanged; diff --git a/Xamarin.Forms.Platform.Android/Renderers/CarouselViewRenderer.cs b/Xamarin.Forms.Platform.Android/Renderers/CarouselViewRenderer.cs index b44ef3e..12a1157 100644 --- a/Xamarin.Forms.Platform.Android/Renderers/CarouselViewRenderer.cs +++ b/Xamarin.Forms.Platform.Android/Renderers/CarouselViewRenderer.cs @@ -267,7 +267,7 @@ namespace Xamarin.Forms.Platform.Android internal override bool CanScrollHorizontally => true; internal override bool CanScrollVertically => false; - internal override IntRectangle GetBounds(int originPosition, RecyclerView.State state) => + internal override IntRectangle GetBounds(int originPosition, State state) => new IntRectangle( LayoutItem(originPosition, 0).Location, new IntSize(_itemSize.Width * state.ItemCount, _itemSize.Height) @@ -583,7 +583,7 @@ namespace Xamarin.Forms.Platform.Android } // initialize properties - VisualElementController.SetValueFromRenderer(CarouselView.PositionProperty, 0); + _position = Element.Position; // initialize events Element.CollectionChanged += OnCollectionChanged; @@ -778,10 +778,7 @@ namespace Xamarin.Forms.Platform.Android AdapterChangeType _adapterChangeType; #endregion - public PhysicalLayoutManager( - Context context, - VirtualLayoutManager virtualLayout, - int positionOrigin) + internal PhysicalLayoutManager(Context context, VirtualLayoutManager virtualLayout, int positionOrigin) { _positionOrigin = positionOrigin; _context = context; @@ -793,7 +790,7 @@ namespace Xamarin.Forms.Platform.Android _scroller = new SeekAndSnapScroller( context: context, vectorToPosition: adapterPosition => { - var end = virtualLayout.LayoutItem(positionOrigin, adapterPosition).Center(); + var end = virtualLayout.LayoutItem(_positionOrigin, adapterPosition).Center(); var begin = Viewport.Center(); return end - begin; } @@ -853,24 +850,24 @@ namespace Xamarin.Forms.Platform.Android base.Dispose(disposing); } - public event Action OnAppearing; - public event Action OnBeginScroll; - public event Action OnDisappearing; - public event Action OnEndScroll; + internal event Action OnAppearing; + internal event Action OnBeginScroll; + internal event Action OnDisappearing; + internal event Action OnEndScroll; - public IntVector Velocity => _samples.Aggregate((o, a) => o + a) / _samples.Count; - public void Layout(int width, int height) + internal IntVector Velocity => _samples.Aggregate((o, a) => o + a) / _samples.Count; + internal void Layout(int width, int height) { // e.g. when rotated the width and height are updated the virtual layout will // need to resize and provide a new viewport offset given the current one. _virtualLayout.Layout(_positionOrigin, new IntSize(width, height), ref _locationOffset); } - public IntRectangle Viewport => Rectangle + _locationOffset; - public IEnumerable VisiblePositions() + internal IntRectangle Viewport => Rectangle + _locationOffset; + internal IEnumerable VisiblePositions() { return _visibleAdapterPosition; } - public IEnumerable Views() + internal IEnumerable Views() { return _viewByAdaptorPosition.Values; } diff --git a/Xamarin.Forms.Platform.iOS/Renderers/CarouselViewRenderer.cs b/Xamarin.Forms.Platform.iOS/Renderers/CarouselViewRenderer.cs index d07e9d8..d52feb7 100644 --- a/Xamarin.Forms.Platform.iOS/Renderers/CarouselViewRenderer.cs +++ b/Xamarin.Forms.Platform.iOS/Renderers/CarouselViewRenderer.cs @@ -48,7 +48,10 @@ namespace Xamarin.Forms.Platform.iOS #endregion #region Fields - CarouselViewController.Layout _layout; + // As on Android, ScrollToPostion from 0 to 2 should not raise OnPositionChanged for 1 + // Tracking the _targetPosition allows for skipping events for intermediate positions + int? _targetPosition; + int _position; CarouselViewController _controller; #endregion @@ -73,19 +76,13 @@ namespace Xamarin.Forms.Platform.iOS return; _controller = new CarouselViewController( - renderer: this, - layout: _layout = new CarouselViewController.Layout( - UICollectionViewScrollDirection.Horizontal - ), - originPosition: Element.Position + renderer: this, + initialPosition: Element.Position ); // hook up on position changed event // not ideal; the event is raised upon releasing the swipe instead of animation completion - _layout.OnSwipeOffsetChosen += o => OnPositionChange(o); - - // hook up crud events - Element.CollectionChanged += OnCollectionChanged; + _controller.OnWillDisplayCell += o => OnPositionChange(o); // populate cache SetNativeControl(_controller.CollectionView); @@ -96,22 +93,28 @@ namespace Xamarin.Forms.Platform.iOS var item = Controller.GetItem(position); Controller.SendSelectedItemChanged(item); } - bool OnPositionChange(int position) + void OnPositionChange(int position) { if (position == _position) - return false; + return; + + if (_targetPosition != null && position != _targetPosition) + return; + _targetPosition = null; _position = position; + Element.Position = _position; Controller.SendSelectedPositionChanged(position); OnItemChange(position); - return true; + return; } void ScrollToPosition(int position, bool animated = true) { - if (!OnPositionChange(position)) + if (position == _position) return; + _targetPosition = position; _controller.ScrollToPosition(position, animated); } void OnCollectionChanged(object source, NotifyCollectionChangedEventArgs e) @@ -184,7 +187,7 @@ namespace Xamarin.Forms.Platform.iOS protected override void OnElementPropertyChanged(object sender, PropertyChangedEventArgs e) { - if (e.PropertyName == "Position") + if (e.PropertyName == "Position" && _position != Element.Position) // not ideal; the event is raised before the animation to move completes (or even starts) ScrollToPosition(Element.Position); @@ -193,7 +196,27 @@ namespace Xamarin.Forms.Platform.iOS protected override void OnElementChanged(ElementChangedEventArgs e) { base.OnElementChanged(e); - Initialize(); + + CarouselView oldElement = e.OldElement; + CarouselView newElement = e.NewElement; + if (oldElement != null) + { + e.OldElement.CollectionChanged -= OnCollectionChanged; + } + + if (newElement != null) + { + if (Control == null) + { + Initialize(); + } + + // initialize properties + _position = Element.Position; + + // hook up crud events + Element.CollectionChanged += OnCollectionChanged; + } } public override SizeRequest GetDesiredSize(double widthConstraint, double heightConstraint) @@ -204,77 +227,15 @@ namespace Xamarin.Forms.Platform.iOS internal sealed class CarouselViewController : UICollectionViewController { - internal new sealed class Layout : UICollectionViewFlowLayout - { + new sealed class Layout : UICollectionViewFlowLayout { static readonly nfloat ZeroMinimumInteritemSpacing = 0; static readonly nfloat ZeroMinimumLineSpacing = 0; - int _width; - - public Layout(UICollectionViewScrollDirection scrollDirection) - { + public Layout(UICollectionViewScrollDirection scrollDirection) { ScrollDirection = scrollDirection; MinimumInteritemSpacing = ZeroMinimumInteritemSpacing; MinimumLineSpacing = ZeroMinimumLineSpacing; } - - public Action OnSwipeOffsetChosen; - - public override UICollectionViewLayoutAttributes InitialLayoutAttributesForAppearingItem(NSIndexPath itemIndexPath) - { - return base.InitialLayoutAttributesForAppearingItem(itemIndexPath); - } - public override UICollectionViewLayoutAttributes FinalLayoutAttributesForDisappearingItem(NSIndexPath itemIndexPath) - { - return base.FinalLayoutAttributesForDisappearingItem(itemIndexPath); - } - public override UICollectionViewLayoutAttributes[] LayoutAttributesForElementsInRect(RectangleF rect) - { - // couldn't figure a way to use these values to compute when an element appeared to disappeared. YMMV - var result = base.LayoutAttributesForElementsInRect(rect); - foreach (var item in result) - { - var index = item.IndexPath; - var category = item.RepresentedElementCategory; - var kind = item.RepresentedElementKind; - - var hidden = item.Hidden; - var bounds = item.Bounds; - var frame = item.Frame; - var center = item.Center; - - _width = (int)item.Bounds.Width; - } - return result; - } - public override SizeF CollectionViewContentSize - { - get - { - var result = base.CollectionViewContentSize; - return result; - } - } - public override NSIndexPath GetTargetIndexPathForInteractivelyMovingItem(NSIndexPath previousIndexPath, PointF position) - { - var result = base.GetTargetIndexPathForInteractivelyMovingItem(previousIndexPath, position); - return result; - } - public override PointF TargetContentOffsetForProposedContentOffset(PointF proposedContentOffset) - { - var result = base.TargetContentOffsetForProposedContentOffset(proposedContentOffset); - return result; - } - public override PointF TargetContentOffset(PointF proposedContentOffset, PointF scrollingVelocity) - { - var result = base.TargetContentOffset(proposedContentOffset, scrollingVelocity); - OnSwipeOffsetChosen?.Invoke((int)result.X / _width); - return result; - } - public override bool ShouldInvalidateLayoutForBoundsChange(RectangleF newBounds) - { - return true; - } } sealed class Cell : UICollectionViewCell { @@ -283,12 +244,6 @@ namespace Xamarin.Forms.Platform.iOS IVisualElementRenderer _renderer; View _view; - [Export("initWithFrame:")] - internal Cell(RectangleF frame) : base(frame) - { - _position = -1; - } - void Bind(object item, int position) { //if (position != this.position) @@ -300,6 +255,11 @@ namespace Xamarin.Forms.Platform.iOS _controller.BindView(_view, item); } + [Export("initWithFrame:")] + internal Cell(RectangleF frame) : base(frame) + { + _position = -1; + } internal void Initialize(IItemViewController controller, object itemType, object item, int position) { _position = position; @@ -327,7 +287,6 @@ namespace Xamarin.Forms.Platform.iOS } public Action OnBind; - public override void LayoutSubviews() { base.LayoutSubviews(); @@ -337,21 +296,19 @@ namespace Xamarin.Forms.Platform.iOS } readonly Dictionary _typeIdByType; - UICollectionViewLayout _layout; CarouselViewRenderer _renderer; int _nextItemTypeId; - int _originPosition; - - public Action OnBind; - public Action OnSwipeTargetChosen; + int _initialPosition; - public CarouselViewController(CarouselViewRenderer renderer, UICollectionViewLayout layout, int originPosition) : base(layout) + internal CarouselViewController( + CarouselViewRenderer renderer, + int initialPosition) + : base(new Layout(UICollectionViewScrollDirection.Horizontal)) { _renderer = renderer; _typeIdByType = new Dictionary(); _nextItemTypeId = 0; - _layout = layout; - _originPosition = originPosition; + _initialPosition = initialPosition; } CarouselViewRenderer Renderer => _renderer; @@ -367,15 +324,19 @@ namespace Xamarin.Forms.Platform.iOS return collectionView.Frame.Size; } + internal Action OnBind; + internal Action OnWillDisplayCell; + public override void WillDisplayCell(UICollectionView collectionView, UICollectionViewCell cell, NSIndexPath indexPath) { - if (_originPosition == 0) + if (_initialPosition != 0) { + ScrollToPosition(_initialPosition, false); + _initialPosition = 0; return; + } - // Ideally position zero would not be rendered in memory however it is. - // Thankfully, position zero is not displyed; position originPosition is rendered and displayed. - ScrollToPosition(_originPosition, false); - _originPosition = 0; + var index = indexPath.Row; + OnWillDisplayCell?.Invoke(index); } public override nint NumberOfSections(UICollectionView collectionView) { @@ -397,8 +358,8 @@ namespace Xamarin.Forms.Platform.iOS { var index = indexPath.Row; - if (_originPosition != 0) - index = _originPosition; + if (_initialPosition != 0) + index = _initialPosition; var item = Controller.GetItem(index); var itemType = Controller.GetItemType(item); @@ -420,18 +381,18 @@ namespace Xamarin.Forms.Platform.iOS return cell; } - public void ReloadData() => CollectionView.ReloadData(); - public void ReloadItems(IEnumerable positions) + internal void ReloadData() => CollectionView.ReloadData(); + internal void ReloadItems(IEnumerable positions) { var indices = positions.Select(o => NSIndexPath.FromRowSection(o, 0)).ToArray(); CollectionView.ReloadItems(indices); } - public void DeleteItems(IEnumerable positions) + internal void DeleteItems(IEnumerable positions) { var indices = positions.Select(o => NSIndexPath.FromRowSection(o, 0)).ToArray(); CollectionView.DeleteItems(indices); } - public void MoveItem(int oldPosition, int newPosition) + internal void MoveItem(int oldPosition, int newPosition) { base.MoveItem( CollectionView, @@ -439,7 +400,7 @@ namespace Xamarin.Forms.Platform.iOS NSIndexPath.FromRowSection(newPosition, 0) ); } - public void ScrollToPosition(int position, bool animated = true) + internal void ScrollToPosition(int position, bool animated = true) { CollectionView.ScrollToItem( indexPath: NSIndexPath.FromRowSection(position, 0),