Remove math on scroll view container for RTL (#3299)

* fixes #3000 remove math on scroll view container for RTL
- setup RTL scrollviews to all start scrolled all the way to the right

* * formatting and renaming fixes

* - don't move scroll if not set to RTL

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Issue3000.cs

@@ -0,0 +1,125 @@
+using Xamarin.Forms.CustomAttributes;
+using Xamarin.Forms.Internals;
+
+
+#if UITEST
+using Xamarin.UITest;
+using NUnit.Framework;
+using Xamarin.Forms.Core.UITests;
+#endif
+
+namespace Xamarin.Forms.Controls.Issues
+{
+	[Preserve(AllMembers = true)]
+	[Issue(IssueTracker.Github, 3000, "Horizontal ScrollView breaks scrolling when flowdirection is set to rtl")]
+#if UITEST
+	[NUnit.Framework.Category(UITestCategories.ScrollView)]
+#endif
+	public class Issue3000 : TestContentPage
+	{
+		const string kSuccess = "Success";
+
+		protected override void Init()
+		{
+			ScrollView view = new ScrollView();
+			StackLayout parent = new StackLayout();
+			Label instructions = new Label() { Text = "Scroll X should not be zero Scroll Y should be zero" };
+			Label scrollPositions = new Label();
+			Label outcome = new Label();
+
+			parent.Children.Add(instructions);
+			parent.Children.Add(scrollPositions);
+			parent.Children.Add(outcome);
+
+			view.Scrolled += (_, __) =>
+			{
+				if (outcome.Text == kSuccess)
+				{
+					return;
+				}
+
+				scrollPositions.Text = $"ScrollX: {view.ScrollX} ScrollY: {view.ScrollY}";
+				if (view.ScrollY == 0 && view.ScrollX > 0)
+				{
+					outcome.Text = kSuccess;
+				}
+				else
+				{
+					outcome.Text = "Fail";
+				}
+			};
+
+			view.Orientation = ScrollOrientation.Both;
+
+			StackLayout layout = new StackLayout();
+			layout.Orientation = StackOrientation.Horizontal;
+			layout.Children.Add(new Label() { Text = "LEFT" });
+			for (int i = 0; i < 80; i++)
+				layout.Children.Add(new Image() { BackgroundColor = Color.Pink, Source = "coffee.png" });
+			layout.Children.Add(new Label() { Text = "RIGHT" });
+
+
+
+			StackLayout layoutDown = new StackLayout();
+			for (int i = 0; i < 80; i++)
+				layoutDown.Children.Add(new Image() { BackgroundColor = Color.Pink, Source = "coffee.png" });
+
+			view.FlowDirection = FlowDirection.RightToLeft;
+			parent.Children.Insert(0, new Button()
+			{
+				Text = "click me please",
+				Command = new Command(() =>
+				{
+					if (view.FlowDirection == FlowDirection.LeftToRight)
+					{
+						view.FlowDirection = FlowDirection.RightToLeft;
+					}
+					else
+					{
+						view.FlowDirection = FlowDirection.LeftToRight;
+					}
+				})
+			});
+
+			parent.Children.Insert(0, new Button()
+			{
+				Text = "reset this view",
+				Command = new Command(() =>
+				{
+					Application.Current.MainPage = new Issue3000();
+				})
+			});
+
+			parent.Children.Insert(0, new Label()
+			{
+				Text = "right to left text",
+			});
+
+			parent.Children.Insert(0, new Label()
+			{
+				Text = "left to right text"
+			});
+
+			view.Content = new StackLayout()
+			{
+				Children =
+				{
+					layout, layoutDown
+				}
+			};
+
+			parent.Children.Add(view);
+			Content = parent;
+		}
+
+
+#if UITEST
+		[Test]
+		public void RtlScrollViewStartsScrollToRight()
+		{
+			RunningApp.WaitForElement(kSuccess);
+		}
+#endif
+
+	}
+}

Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems

@@ -245,6 +245,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Issue1556.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1799.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue1931.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Issue3000.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3053.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue2617.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Issue3087.cs" />

Xamarin.Forms.Core/Cells/Cell.cs

@@ -36,6 +36,8 @@ namespace Xamarin.Forms
 			}
 		}
 
+		bool IFlowDirectionController.ApplyEffectiveFlowDirectionToChildContainer => true;
+
 		IFlowDirectionController FlowController => this;
 
 		public IList<MenuItem> ContextActions

Xamarin.Forms.Core/IFlowDirectionController.cs

@@ -7,5 +7,7 @@ namespace Xamarin.Forms
 		double Width { get; }
 
 		void NotifyFlowDirectionChanged();
+
+		bool ApplyEffectiveFlowDirectionToChildContainer { get; }
 	}
 }

Xamarin.Forms.Core/Layout.cs

@@ -137,7 +137,7 @@ namespace Xamarin.Forms
 		{
 			var parent = child.Parent as IFlowDirectionController;
 			bool isRightToLeft = false;
-			if (parent != null && (isRightToLeft = parent.EffectiveFlowDirection.IsRightToLeft()))
+			if (parent != null && (isRightToLeft = parent.ApplyEffectiveFlowDirectionToChildContainer && parent.EffectiveFlowDirection.IsRightToLeft()))
 				region = new Rectangle(parent.Width - region.Right, region.Y, region.Width, region.Height);
 
 			var view = child as View;
@@ -280,7 +280,7 @@ namespace Xamarin.Forms
 		{
 			var parent = child.Parent as IFlowDirectionController;
 			bool isRightToLeft = false;
-			if (parent != null && (isRightToLeft = parent.EffectiveFlowDirection.IsRightToLeft()))
+			if (parent != null && (isRightToLeft = parent.ApplyEffectiveFlowDirectionToChildContainer && parent.EffectiveFlowDirection.IsRightToLeft()))
 				region = new Rectangle(parent.Width - region.Right, region.Y, region.Width, region.Height);
 
 			if (region.Size != childSizeRequest.Request)

Xamarin.Forms.Core/ScrollView.cs

@@ -8,7 +8,7 @@ namespace Xamarin.Forms
 {
 	[ContentProperty("Content")]
 	[RenderWith(typeof(_ScrollViewRenderer))]
-	public class ScrollView : Layout, IScrollViewController, IElementConfiguration<ScrollView>
+	public class ScrollView : Layout, IScrollViewController, IElementConfiguration<ScrollView>, IFlowDirectionController
 	{
 		public static readonly BindableProperty OrientationProperty = BindableProperty.Create("Orientation", typeof(ScrollOrientation), typeof(ScrollView), ScrollOrientation.Vertical);
 
@@ -149,9 +149,7 @@ namespace Xamarin.Forms
 			ScrollX = x;
 			ScrollY = y;
 
-			EventHandler<ScrolledEventArgs> handler = Scrolled;
-			if (handler != null)
-				handler(this, new ScrolledEventArgs(x, y));
+			Scrolled?.Invoke(this, new ScrolledEventArgs(x, y));
 		}
 
 		public event EventHandler<ScrolledEventArgs> Scrolled;
@@ -184,6 +182,8 @@ namespace Xamarin.Forms
 			return _scrollCompletionSource.Task;
 		}
 
+		bool IFlowDirectionController.ApplyEffectiveFlowDirectionToChildContainer => false;
+		
 		protected override void LayoutChildren(double x, double y, double width, double height)
 		{
 			if (_content != null)

Xamarin.Forms.Core/VisualElement.cs

@@ -882,6 +882,8 @@ namespace Xamarin.Forms
 				unFocus(this, new FocusEventArgs(this, false));
 		}
 
+		bool IFlowDirectionController.ApplyEffectiveFlowDirectionToChildContainer => true;
+
 		void IFlowDirectionController.NotifyFlowDirectionChanged()
 		{
 			SetFlowDirectionFromParent(this);

Xamarin.Forms.Platform.Android/ContextExtensions.cs

@@ -42,6 +42,9 @@ namespace Xamarin.Forms.Platform.Android
 		public static bool HasRtlSupport(this Context self) =>
 			(self.ApplicationInfo.Flags & AApplicationInfoFlags.SupportsRtl) == AApplicationInfoFlags.SupportsRtl;
 
+		public static int TargetSdkVersion(this Context self) =>
+			(int)self.ApplicationInfo.TargetSdkVersion;
+
 		internal static double GetThemeAttributeDp(this Context self, int resource)
 		{
 			using (var value = new TypedValue())

Xamarin.Forms.Platform.Android/Extensions/FlowDirectionExtensions.cs

@@ -28,6 +28,7 @@ namespace Xamarin.Forms.Platform.Android
 			if (view == null || controller == null || (int)Build.VERSION.SdkInt < 17)
 				return;
 
+			// if android:targetSdkVersion < 17 setting these has no effect
 			if (controller.EffectiveFlowDirection.IsRightToLeft())
 				view.LayoutDirection = ALayoutDirection.Rtl;
 			else if (controller.EffectiveFlowDirection.IsLeftToRight())

Xamarin.Forms.Platform.Android/Renderers/ScrollViewRenderer.cs

@@ -25,6 +25,8 @@ namespace Xamarin.Forms.Platform.Android
 		int _previousBottom;
 		bool _isEnabled;
 		bool _disposed;
+		LayoutDirection _prevLayoutDirection = LayoutDirection.Ltr;
+		bool _checkedForRtlScroll = false;
 
 		public ScrollViewRenderer(Context context) : base(context)
 		{
@@ -93,6 +95,7 @@ namespace Xamarin.Forms.Platform.Android
 				UpdateIsEnabled();
 				UpdateHorizontalScrollBarVisibility();
 				UpdateVerticalScrollBarVisibility();
+				UpdateFlowDirection();
 
 				element.SendViewInitialized(this);
 
@@ -103,6 +106,22 @@ namespace Xamarin.Forms.Platform.Android
 			EffectUtilities.RegisterEffectControlProvider(this, oldElement, element);
 		}
 
+		void UpdateFlowDirection()
+		{
+			if (Element is IVisualElementController controller)
+			{
+				var flowDirection = controller.EffectiveFlowDirection.IsLeftToRight()
+					? LayoutDirection.Ltr
+					: LayoutDirection.Rtl;
+
+				if (_prevLayoutDirection != flowDirection && _hScrollView != null)
+				{
+					_prevLayoutDirection = flowDirection;
+					_hScrollView.LayoutDirection = flowDirection;
+				}
+			}
+		}
+
 		public VisualElementTracker Tracker { get; private set; }
 
 		public void UpdateLayout()
@@ -229,12 +248,19 @@ namespace Xamarin.Forms.Platform.Android
 			base.OnLayout(changed, left, top, right, bottom);
 			if (_view.Content != null && _hScrollView != null)
 				_hScrollView.Layout(0, 0, right - left, Math.Max(bottom - top, (int)Context.ToPixels(_view.Content.Height)));
-			else if(_view.Content != null && requestContainerLayout)
+			else if (_view.Content != null && requestContainerLayout)
 				_container?.RequestLayout();
+
+			// if the target sdk >= 17 then setting the LayoutDirection on the scroll view natively takes care of the scroll
+			if (Context.TargetSdkVersion() < 17 && !_checkedForRtlScroll && _hScrollView != null && Element is IVisualElementController controller && controller.EffectiveFlowDirection.IsRightToLeft())
+				_hScrollView.ScrollX = _container.MeasuredWidth - _hScrollView.MeasuredWidth - _hScrollView.ScrollX;
+
+			_checkedForRtlScroll = true;
 		}
 
 		protected override void OnScrollChanged(int l, int t, int oldl, int oldt)
 		{
+			_checkedForRtlScroll = true;
 			base.OnScrollChanged(l, t, oldl, oldt);
 			var context = Context;
 			UpdateScrollPosition(context.FromPixels(l), context.FromPixels(t));
@@ -293,6 +319,8 @@ namespace Xamarin.Forms.Platform.Android
 				UpdateHorizontalScrollBarVisibility();
 			else if (e.PropertyName == ScrollView.VerticalScrollBarVisibilityProperty.PropertyName)
 				UpdateVerticalScrollBarVisibility();
+			else if (e.PropertyName == VisualElement.FlowDirectionProperty.PropertyName)
+				UpdateFlowDirection();
 		}
 
 		void UpdateIsEnabled()
@@ -312,6 +340,8 @@ namespace Xamarin.Forms.Platform.Android
 
 		async void OnScrollToRequested(object sender, ScrollToRequestedEventArgs e)
 		{
+			_checkedForRtlScroll = true;
+
 			if (!_isAttached)
 			{
 				return;
@@ -417,7 +447,10 @@ namespace Xamarin.Forms.Platform.Android
 			if (_view.Orientation == ScrollOrientation.Horizontal || _view.Orientation == ScrollOrientation.Both)
 			{
 				if (_hScrollView == null)
+				{
 					_hScrollView = new AHorizontalScrollView(Context, this);
+					UpdateFlowDirection();
+				}
 
 				((AHorizontalScrollView)_hScrollView).IsBidirectional = _isBidirectional = _view.Orientation == ScrollOrientation.Both;
 

Xamarin.Forms.Platform.UAP/ScrollViewRenderer.cs

@@ -11,6 +11,7 @@ namespace Xamarin.Forms.Platform.UWP
 	public class ScrollViewRenderer : ViewRenderer<ScrollView, ScrollViewer>
 	{
 		VisualElement _currentView;
+		bool _checkedForRtlScroll = false;
 
 		public ScrollViewRenderer()
 		{
@@ -40,11 +41,7 @@ namespace Xamarin.Forms.Platform.UWP
 
 		protected override void Dispose(bool disposing)
 		{
-			if (Control != null)
-			{
-				Control.ViewChanged -= OnViewChanged;
-			}
-
+			CleanUp(Element, Control);
 			base.Dispose(disposing);
 		}
 
@@ -62,14 +59,25 @@ namespace Xamarin.Forms.Platform.UWP
 			return result;
 		}
 
+		void CleanUp(ScrollView scrollView, ScrollViewer scrollViewer)
+		{
+			if (scrollView != null)
+				scrollView.ScrollToRequested -= OnScrollToRequested;
+
+			if (scrollViewer != null)
+			{
+				scrollViewer.ViewChanged -= OnViewChanged;
+				if (scrollViewer.Content is FrameworkElement element)
+				{
+					element.LayoutUpdated -= SetInitialRtlPosition;
+				}
+			}
+		}
+
 		protected override void OnElementChanged(ElementChangedEventArgs<ScrollView> e)
 		{
 			base.OnElementChanged(e);
-
-			if (e.OldElement != null)
-			{
-				e.OldElement.ScrollToRequested -= OnScrollToRequested;
-			}
+			CleanUp(e.OldElement, Control);
 
 			if (e.NewElement != null)
 			{
@@ -88,7 +96,7 @@ namespace Xamarin.Forms.Platform.UWP
 
 				UpdateOrientation();
 
-				LoadContent();
+				UpdateContent();
 			}
 		}
 
@@ -97,7 +105,7 @@ namespace Xamarin.Forms.Platform.UWP
 			base.OnElementPropertyChanged(sender, e);
 
 			if (e.PropertyName == "Content")
-				LoadContent();
+				UpdateContent();
 			else if (e.PropertyName == Layout.PaddingProperty.PropertyName)
 				UpdateMargins();
 			else if (e.PropertyName == ScrollView.OrientationProperty.PropertyName)
@@ -108,28 +116,31 @@ namespace Xamarin.Forms.Platform.UWP
 				UpdateHorizontalScrollBarVisibility();
 		}
 
-		void LoadContent()
+		void UpdateContent()
 		{
 			if (_currentView != null)
-			{
 				_currentView.Cleanup();
-			}
+
+			if (Control?.Content is FrameworkElement frameworkElement)
+				frameworkElement.LayoutUpdated -= SetInitialRtlPosition;
 
 			_currentView = Element.Content;
 
 			IVisualElementRenderer renderer = null;
 			if (_currentView != null)
-			{
 				renderer = _currentView.GetOrCreateRenderer();
-			}
 
 			Control.Content = renderer != null ? renderer.ContainerElement : null;
 
 			UpdateMargins();
+			if(renderer.ContainerElement != null)
+				renderer.ContainerElement.LayoutUpdated += SetInitialRtlPosition;
 		}
 
 		async void OnScrollToRequested(object sender, ScrollToRequestedEventArgs e)
 		{
+			ClearRtlScrollCheck();
+
 			// Adding items into the view while scrolling to the end can cause it to fail, as
 			// the items have not actually been laid out and return incorrect scroll position
 			// values. The ScrollViewRenderer for Android does something similar by waiting up
@@ -161,9 +172,39 @@ namespace Xamarin.Forms.Platform.UWP
 			}
 			Element.SendScrollFinished();
 		}
+				
+		void SetInitialRtlPosition(object sender, object e)
+		{
+			if (Control == null) return;
+
+			if (Control.ActualWidth <= 0 || _checkedForRtlScroll || Control.Content == null)
+				return;
+
+			if (Element is IVisualElementController controller && controller.EffectiveFlowDirection.IsLeftToRight())
+			{
+				ClearRtlScrollCheck();
+				return;
+			}
+
+			var element = (Control.Content as FrameworkElement);
+			if (element.ActualWidth == Control.ActualWidth)
+				return;
+
+			ClearRtlScrollCheck();
+			Control.ChangeView(element.ActualWidth, 0, null, true);
+		}
+
+		void ClearRtlScrollCheck()
+		{
+			_checkedForRtlScroll = true;
+			var element = (Control.Content as FrameworkElement);
+			if (element != null)
+				element.LayoutUpdated -= SetInitialRtlPosition;
+		}
 
 		void OnViewChanged(object sender, ScrollViewerViewChangedEventArgs e)
 		{
+			ClearRtlScrollCheck();
 			Element.SetScrolledPosition(Control.HorizontalOffset, Control.VerticalOffset);
 
 			if (!e.IsIntermediate)
@@ -207,7 +248,7 @@ namespace Xamarin.Forms.Platform.UWP
 
 		UwpScrollBarVisibility ScrollBarVisibilityToUwp(ScrollBarVisibility visibility)
 		{
-			switch(visibility)
+			switch (visibility)
 			{
 				case ScrollBarVisibility.Always:
 					return UwpScrollBarVisibility.Visible;

Xamarin.Forms.Platform.iOS/Renderers/ScrollViewRenderer.cs

@@ -18,6 +18,8 @@ namespace Xamarin.Forms.Platform.iOS
 		RectangleF _previousFrame;
 		ScrollToRequestedEventArgs _requestedScroll;
 		VisualElementTracker _tracker;
+		bool _checkedForRtlScroll = false;
+		bool _previousLTR = true;
 
 		public ScrollViewRenderer() : base(RectangleF.Empty)
 		{
@@ -111,11 +113,18 @@ namespace Xamarin.Forms.Platform.iOS
 		{
 			base.LayoutSubviews();
 
-			if (_requestedScroll != null && Superview != null)
+			if(Superview != null)
 			{
-				var request = _requestedScroll;
-				_requestedScroll = null;
-				OnScrollToRequested(this, request);
+				if (_requestedScroll != null)
+				{
+					var request = _requestedScroll;
+					_requestedScroll = null;
+					OnScrollToRequested(this, request);
+				}
+				else
+				{
+					UpdateFlowDirection();
+				}
 			}
 
 			if (_previousFrame != Frame)
@@ -125,6 +134,25 @@ namespace Xamarin.Forms.Platform.iOS
 			}
 		}
 
+		void UpdateFlowDirection()
+		{
+			if (Superview == null || _requestedScroll != null || _checkedForRtlScroll)
+				return;
+
+			if (Element is IVisualElementController controller && ScrollView.Orientation != ScrollOrientation.Vertical)
+			{
+				var isLTR = controller.EffectiveFlowDirection.IsLeftToRight();
+				if (_previousLTR != isLTR)
+				{
+					_previousLTR = isLTR;
+					_checkedForRtlScroll = true;
+					SetContentOffset(new PointF((nfloat)(ScrollView.Content.Width - ScrollView.Width - ContentOffset.X), 0), false);
+				}
+			}
+
+			_checkedForRtlScroll = true;
+		}
+
 		protected override void Dispose(bool disposing)
 		{
 			if (disposing)
@@ -154,12 +182,7 @@ namespace Xamarin.Forms.Platform.iOS
 			base.Dispose(disposing);
 		}
 
-		protected virtual void OnElementChanged(VisualElementChangedEventArgs e)
-		{
-			var changed = ElementChanged;
-			if (changed != null)
-				changed(this, e);
-		}
+		protected virtual void OnElementChanged(VisualElementChangedEventArgs e) => ElementChanged?.Invoke(this, e); 
 
 		void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
 		{
@@ -219,6 +242,8 @@ namespace Xamarin.Forms.Platform.iOS
 
 		void OnScrollToRequested(object sender, ScrollToRequestedEventArgs e)
 		{
+			_checkedForRtlScroll = true;
+
 			if (Superview == null)
 			{
 				_requestedScroll = e;