fix(SplitView): Ensure clicking outside SplitView pane closes the pane (microsoft#747)

rozele · web-flow · commit 6ae4661404c2 · 2016-10-06T15:19:00.000-04:00
A recent change to the touch handler to support cases where overlays that did not support any pointer events were used (so the underlying React view that did support pointer events could be reached) ignored the fact that some clicks simply don't belong to the React hierarchy (e.g., native app overlays, as is the case for SplitView). This change first checks that the original pointer target actually belongs to the React before calculating the target using pointer events logic. Fixes microsoft#746
diff --git a/ReactWindows/ReactNative/Touch/TouchHandler.cs b/ReactWindows/ReactNative/Touch/TouchHandler.cs
@@ -4,7 +4,6 @@
 using ReactNative.UIManager.Events;
 using System;
 using System.Collections.Generic;
-using Windows.Foundation;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Input;
 using Windows.UI.Xaml.Media;
@@ -47,7 +46,7 @@ private void OnPointerPressed(object sender, PointerRoutedEventArgs e)
                 throw new InvalidOperationException("A pointer with this ID already exists.");
             }
 
-            var reactView = GetReactViewFromPoint(e.GetCurrentPoint(_view).Position);
+            var reactView = GetReactViewTarget(e);
             if (reactView != null && _view.CapturePointer(e.Pointer))
             {
                 var reactTag = reactView.GetReactCompoundView().GetReactTagAtPoint(reactView,
@@ -124,16 +123,24 @@ private int IndexOfPointerWithId(uint pointerId)
             return -1;
         }
 
-        private UIElement GetReactViewFromPoint(Point point)
+        private UIElement GetReactViewTarget(PointerRoutedEventArgs e)
         {
+            // If the target is not a child of the root view, then this pointer
+            // event does not belong to React.
+            if (!RootViewHelper.IsReactSubview(e.OriginalSource as DependencyObject))
+            {
+                return null;
+            }
+
+            var point = e.GetCurrentPoint(_view).Position;
             var sources = VisualTreeHelper.FindElementsInHostCoordinates(point, _view);
 
             // Get the first React view that does not have pointer events set
             // to 'none' or 'box-none', and is not a child of a view with 
             // 'box-only' or 'none' settings for pointer events.
 
             // TODO: use pooled data structure
-            var isBoxOnlyCache = new Dictionary<UIElement, bool>();
+            var isBoxOnlyCache = new Dictionary<DependencyObject, bool>();
             foreach (var source in sources)
             {
                 if (!source.HasTag())
@@ -192,7 +199,7 @@ private void DispatchTouchEvent(TouchEventType touchEventType, List<ReactPointer
                 .DispatchEvent(touchEvent);
         }
 
-        private static bool IsBoxOnlyWithCache(IEnumerable<UIElement> hierarchy, IDictionary<UIElement, bool> cache)
+        private static bool IsBoxOnlyWithCache(IEnumerable<DependencyObject> hierarchy, IDictionary<DependencyObject, bool> cache)
         {
             var enumerator = hierarchy.GetEnumerator();
 
@@ -205,7 +212,7 @@ private static bool IsBoxOnlyWithCache(IEnumerable<UIElement> hierarchy, IDictio
             return IsBoxOnlyWithCacheRecursive(enumerator, cache);
         }
 
-        private static bool IsBoxOnlyWithCacheRecursive(IEnumerator<UIElement> enumerator, IDictionary<UIElement, bool> cache)
+        private static bool IsBoxOnlyWithCacheRecursive(IEnumerator<DependencyObject> enumerator, IDictionary<DependencyObject, bool> cache)
         {
             if (!enumerator.MoveNext())
             {
diff --git a/ReactWindows/ReactNative/UIManager/RootViewHelper.cs b/ReactWindows/ReactNative/UIManager/RootViewHelper.cs
@@ -1,5 +1,4 @@
 ﻿using System.Collections.Generic;
-using System.Linq;
 using Windows.UI.Xaml;
 using Windows.UI.Xaml.Media;
 
@@ -40,7 +39,7 @@ public static ReactRootView GetRootView(DependencyObject view)
         /// </summary>
         /// <param name="view">The view.</param>
         /// <returns>The view hierarchy.</returns>
-        public static IEnumerable<UIElement> GetReactViewHierarchy(DependencyObject view)
+        public static IEnumerable<DependencyObject> GetReactViewHierarchy(DependencyObject view)
         {
             var current = view;
             while (true)
@@ -65,6 +64,19 @@ public static IEnumerable<UIElement> GetReactViewHierarchy(DependencyObject view
             }
         }
 
+        /// <summary>
+        /// Checks if the view is part of the React hierarchy.
+        /// </summary>
+        /// <param name="view">The view instance.</param>
+        /// <returns>
+        /// <code>true</code> if the view is part of the hierarchy, otherwise
+        /// <code>false</code>.
+        /// </returns>
+        public static bool IsReactSubview(DependencyObject view)
+        {
+            return GetReactViewHierarchy(view).GetEnumerator().MoveNext();
+        }
+
         private static DependencyObject GetParent(DependencyObject view, bool findRoot)
         {
             //

Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@`
`4`	`4`	`using ReactNative.UIManager.Events;`
`5`	`5`	`using System;`
`6`	`6`	`using System.Collections.Generic;`
`7`		`-using Windows.Foundation;`
`8`	`7`	`using Windows.UI.Xaml;`
`9`	`8`	`using Windows.UI.Xaml.Input;`
`10`	`9`	`using Windows.UI.Xaml.Media;`
`@@ -47,7 +46,7 @@ private void OnPointerPressed(object sender, PointerRoutedEventArgs e)`
`47`	`46`	`throw new InvalidOperationException("A pointer with this ID already exists.");`
`48`	`47`	`}`
`49`	`48`
`50`		`- var reactView = GetReactViewFromPoint(e.GetCurrentPoint(_view).Position);`
	`49`	`+ var reactView = GetReactViewTarget(e);`
`51`	`50`	`if (reactView != null && _view.CapturePointer(e.Pointer))`
`52`	`51`	`{`
`53`	`52`	`var reactTag = reactView.GetReactCompoundView().GetReactTagAtPoint(reactView,`
`@@ -124,16 +123,24 @@ private int IndexOfPointerWithId(uint pointerId)`
`124`	`123`	`return -1;`
`125`	`124`	`}`
`126`	`125`
`127`		`- private UIElement GetReactViewFromPoint(Point point)`
	`126`	`+ private UIElement GetReactViewTarget(PointerRoutedEventArgs e)`
`128`	`127`	`{`
	`128`	`+ // If the target is not a child of the root view, then this pointer`
	`129`	`+ // event does not belong to React.`
	`130`	`+ if (!RootViewHelper.IsReactSubview(e.OriginalSource as DependencyObject))`
	`131`	`+ {`
	`132`	`+ return null;`
	`133`	`+ }`
	`134`	`+`
	`135`	`+ var point = e.GetCurrentPoint(_view).Position;`
`129`	`136`	`var sources = VisualTreeHelper.FindElementsInHostCoordinates(point, _view);`
`130`	`137`
`131`	`138`	`// Get the first React view that does not have pointer events set`
`132`	`139`	`// to 'none' or 'box-none', and is not a child of a view with`
`133`	`140`	`// 'box-only' or 'none' settings for pointer events.`
`134`	`141`
`135`	`142`	`// TODO: use pooled data structure`
`136`		`- var isBoxOnlyCache = new Dictionary<UIElement, bool>();`
	`143`	`+ var isBoxOnlyCache = new Dictionary<DependencyObject, bool>();`
`137`	`144`	`foreach (var source in sources)`
`138`	`145`	`{`
`139`	`146`	`if (!source.HasTag())`
`@@ -192,7 +199,7 @@ private void DispatchTouchEvent(TouchEventType touchEventType, List<ReactPointer`
`192`	`199`	`.DispatchEvent(touchEvent);`
`193`	`200`	`}`
`194`	`201`
`195`		`- private static bool IsBoxOnlyWithCache(IEnumerable<UIElement> hierarchy, IDictionary<UIElement, bool> cache)`
	`202`	`+ private static bool IsBoxOnlyWithCache(IEnumerable<DependencyObject> hierarchy, IDictionary<DependencyObject, bool> cache)`
`196`	`203`	`{`
`197`	`204`	`var enumerator = hierarchy.GetEnumerator();`
`198`	`205`
`@@ -205,7 +212,7 @@ private static bool IsBoxOnlyWithCache(IEnumerable<UIElement> hierarchy, IDictio`
`205`	`212`	`return IsBoxOnlyWithCacheRecursive(enumerator, cache);`
`206`	`213`	`}`
`207`	`214`
`208`		`- private static bool IsBoxOnlyWithCacheRecursive(IEnumerator<UIElement> enumerator, IDictionary<UIElement, bool> cache)`
	`215`	`+ private static bool IsBoxOnlyWithCacheRecursive(IEnumerator<DependencyObject> enumerator, IDictionary<DependencyObject, bool> cache)`
`209`	`216`	`{`
`210`	`217`	`if (!enumerator.MoveNext())`
`211`	`218`	`{`