Fix z indexing when clipped subviews are removed

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

@@ -123,6 +123,8 @@ public class ReactEditText extends AppCompatEditText
   private static final KeyListener sKeyListener = QwertyKeyListener.getInstanceForFullKeyboard();
   private @Nullable EventDispatcher mEventDispatcher;
 
+  private final ReactEditTextClickDetector clickDetector = new ReactEditTextClickDetector(this);
+
   public ReactEditText(Context context) {
     super(context);
     setFocusableInTouchMode(false);
@@ -207,6 +209,13 @@ public boolean onTouchEvent(MotionEvent ev) {
         // Disallow parent views to intercept touch events, until we can detect if we should be
         // capturing these touches or not.
         this.getParent().requestDisallowInterceptTouchEvent(true);
+        clickDetector.handleDown(ev);
+        break;
+      case MotionEvent.ACTION_UP:
+        clickDetector.handleUp(ev);
+        break;
+      case MotionEvent.ACTION_CANCEL:
+        clickDetector.cancelPress();
         break;
       case MotionEvent.ACTION_MOVE:
         if (mDetectScrollMovement) {

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditTextClickDetector.java

@@ -0,0 +1,92 @@
+package com.facebook.react.views.textinput;
+
+import android.os.Build;
+import android.view.MotionEvent;
+import android.view.View;
+
+import androidx.annotation.Nullable;
+
+class ReactEditTextClickDetector {
+
+  private static final long MAX_CLICK_DURATION_MS = 250L;
+  private static final int MAX_CLICK_DISTANCE_DP = 12;
+
+  private final ReactEditText reactEditText;
+  private final float screenDensity;
+
+  @Nullable
+  private TimestampedMotionEvent currentDownEvent;
+
+  public ReactEditTextClickDetector(final ReactEditText reactEditText) {
+    this.reactEditText = reactEditText;
+    screenDensity = reactEditText.getResources().getDisplayMetrics().density;
+  }
+
+  void handleDown(final MotionEvent downEvent) {
+    currentDownEvent = new TimestampedMotionEvent(downEvent);
+  }
+
+  void cancelPress() {
+    currentDownEvent = null;
+  }
+
+  void handleUp(final MotionEvent upEvent) {
+    if (currentDownEvent == null) {
+      return;
+    }
+
+    final TimestampedMotionEvent downEvent = currentDownEvent;
+    currentDownEvent = null;
+
+    // for now, if we're not forcing showing the keyboard on clicks, we don't care if it was a
+    // click. we also early return if the view is not enabled.
+    if (!(forceShowKeyboardOnClicks() && reactEditText.isEnabled())) {
+      return;
+    }
+
+    // make sure the press event was close enough in time
+    final long now = System.currentTimeMillis();
+    final long timeDelta = now - downEvent.timestamp;
+    if (timeDelta > MAX_CLICK_DURATION_MS) {
+      return;
+    }
+
+    // make sure the press event was close enough in distance
+    final float oldX = downEvent.motionEvent.getRawX();
+    final float oldY = downEvent.motionEvent.getRawY();
+    final float newX = upEvent.getRawX();
+    final float newY = upEvent.getRawY();
+
+    // distance = sqrt((x2 − x1)^2 + (y2 − y1)^2)
+    final double distancePx = Math.sqrt(
+      Math.pow((newX - oldX), 2) + Math.pow((newY - oldY), 2)
+    );
+
+    double distanceDp = distancePx / screenDensity;
+    if (distanceDp > MAX_CLICK_DISTANCE_DP) {
+      return;
+    }
+
+    reactEditText.showSoftKeyboard();
+  }
+
+  /**
+   * There is a bug on Android 7/8/9 where clicking the view while it is already
+   * focused does not show the keyboard. On those API levels, we force showing
+   * the keyboard when we detect a click.
+   */
+  private static boolean forceShowKeyboardOnClicks() {
+    return Build.VERSION.SDK_INT <= Build.VERSION_CODES.P;
+  }
+
+  private static class TimestampedMotionEvent {
+
+    final long timestamp;
+    final MotionEvent motionEvent;
+
+    TimestampedMotionEvent(final long timestamp, final MotionEvent motionEvent) {
+      this.timestamp = timestamp;
+      this.motionEvent = motionEvent;
+    }
+  }
+}

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactViewGroup.java

@@ -336,6 +336,8 @@ public void updateClippingRect() {
 
     ReactClippingViewGroupHelper.calculateClippingRect(this, mClippingRect);
     updateClippingToRect(mClippingRect);
+    // see comment at `updateDrawingOrderHelper`.
+    updateDrawingOrderHelper();
   }
 
   private void updateClippingToRect(Rect clippingRect) {
@@ -707,6 +709,46 @@ private ReactViewBackgroundDrawable getOrCreateReactViewBackground() {
     return mReactBackgroundDrawable;
   }
 
+  // NOTE(apkumar)
+  // 
+  // This is added in our (Discord's) fork. 
+  //
+  // Normally, the drawing order helper is informed of new and removed views
+  // via handleAddView and handleRemoveView, called in `addView` and
+  // `removeView` respectively. However, when removeClippedSubviews is true,
+  // `addViewWithSubviewClippingEnabled` is called _instead of_ `addView`,
+  // which _does not_ call into the drawing order helper's handleAddView (with
+  // a similar story for removing views). Because of this, the drawing order
+  // helper is not aware of any child views, and so does not perform any of the
+  // z-indexing logic it normally does.
+  //
+  // There were two ways to fix this: we could either simply call
+  // `handleAddView` in `addViewWithSubviewClippingEnabled` (similar for the
+  // remove view path), or we could explicitly update the drawing order helper
+  // after we attach/remove views as part of the clipping process. Here, we've
+  // opted for the second approach.
+  //
+  // I've opted for the second approach because the drawing helper's `update()`
+  // call looks through the view group's children to figure out which ones have
+  // z indices. This implies that the `handleAddView`/`handleRemoveView` calls
+  // should, semantically, be 1-1 with the view group's children changing (so
+  // that calling `update()` after one of the `handle*` calls does not change
+  // anything). Since `addViewWithSubviewClippingEnabled` doesn't actually add
+  // the view to the children (that happens during the clipping process), I
+  // thought it made more sense to just explicitly update the drawing helper.
+  //
+  // Note that a third option would have been to call `handleAddView` and
+  // `handleRemoveView` in a more fine-grained way during the clipping process,
+  // but that is just slightly more intrusive, and it doesn't really feel like
+  // that would be any sort of performance boost. It's possible that in some
+  // cases there is a meaningful performance boost (otherwise I'm not sure why
+  // the drawing order helper even has the `handle*` API), so if we find that
+  // to be the case, we can be more fine-grained.
+  private void updateDrawingOrderHelper() {
+    getDrawingOrderHelper().update();
+    setChildrenDrawingOrderEnabled(getDrawingOrderHelper().shouldEnableCustomDrawingOrder());
+  }
+
   @Override
   public @Nullable Rect getHitSlopRect() {
     return mHitSlopRect;