[Slider] Keyboard navigation fix, part 2

Resolves #1395

GIT_ORIGIN_REV_ID=7762807c9e2b58c4156267c04ae9fbb080ed8835
PiperOrigin-RevId: 317126080

lib/java/com/google/android/material/slider/BaseSlider.java

@@ -1362,7 +1362,7 @@ private float[] getActiveRange() {
     float right = normalizeValue(max);
 
     // In RTL we draw things in reverse, so swap the left and right range values
-    if (ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL) {
+    if (isRtl()) {
       return new float[] {right, left};
     } else {
       return new float[] {left, right};
@@ -1389,7 +1389,7 @@ private void drawInactiveTrack(@NonNull Canvas canvas, int width, int top) {
    */
   private float normalizeValue(float value) {
     float normalized = (value - valueFrom) / (valueTo - valueFrom);
-    if (ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL) {
+    if (isRtl()) {
       return 1 - normalized;
     }
     return normalized;
@@ -1660,7 +1660,7 @@ private float getValueOfTouchPosition() {
     double position = snapPosition(touchPosition);
 
     // We might need to invert the touch position to get the correct value.
-    if (ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL) {
+    if (isRtl()) {
       position = 1 - position;
     }
     return (float) (position * (valueTo - valueFrom) + valueFrom);
@@ -1829,12 +1829,18 @@ public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
             } else if (event.isShiftPressed()) {
               return moveFocus(-1);
             }
-            // fall through
+            return false;
           case KeyEvent.KEYCODE_DPAD_LEFT:
+            moveFocusInAbsoluteDirection(-1);
+            return true;
           case KeyEvent.KEYCODE_MINUS:
             moveFocus(-1);
             return true;
           case KeyEvent.KEYCODE_DPAD_RIGHT:
+            moveFocusInAbsoluteDirection(1);
+            return true;
+          case KeyEvent.KEYCODE_EQUALS:
+            // Numpad Plus == Shift + Equals, at least in AVD, so fall through.
           case KeyEvent.KEYCODE_PLUS:
             moveFocus(1);
             return true;
@@ -1850,9 +1856,6 @@ public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
         isLongPress |= event.isLongPress();
         Float increment = calculateIncrementForKey(keyCode);
         if (increment != null) {
-          if (ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL) {
-            increment = -increment;
-          }
           float clamped =
               MathUtils.clamp(values.get(activeThumbIdx) + increment, valueFrom, valueTo);
           if (snapActiveThumbToValue(clamped)) {
@@ -1868,7 +1871,7 @@ public boolean onKeyDown(int keyCode, @NonNull KeyEvent event) {
             } else if (event.isShiftPressed()) {
               return moveFocus(-1);
             }
-            // fall through
+            return false;
           case KeyEvent.KEYCODE_DPAD_CENTER:
           case KeyEvent.KEYCODE_ENTER:
             activeThumbIdx = -1;
@@ -1892,15 +1895,22 @@ public boolean onKeyUp(int keyCode, @NonNull KeyEvent event) {
     return super.onKeyUp(keyCode, event);
   }
 
+  final boolean isRtl() {
+    return ViewCompat.getLayoutDirection(this) == ViewCompat.LAYOUT_DIRECTION_RTL;
+  }
+
   /**
-   * Attempts to move focus to next or previous thumb and returns whether the focused thumb changed.
-   * If focused thumb didn't change, we're at the view boundary for specified {@code direction} and
-   * focus should be moved to next or previous view instead.
+   * Attempts to move focus to next or previous thumb <i>independent of layout direction</i>
+   * and returns whether the focused thumb changed.
+   * If focused thumb didn't change, we're at the view boundary for specified {@code direction}
+   * and focus may be moved to next or previous view instead.
+   * @see #moveFocusInAbsoluteDirection(int)
    */
   private boolean moveFocus(int direction) {
-    int oldFocusedThumbIdx = focusedThumbIdx;
-    focusedThumbIdx += direction;
-    focusedThumbIdx = MathUtils.clamp(focusedThumbIdx, 0, values.size() - 1);
+    final int oldFocusedThumbIdx = focusedThumbIdx;
+    // Prevent integer overflow.
+    final long newFocusedThumbIdx = oldFocusedThumbIdx + direction;
+    focusedThumbIdx = (int) MathUtils.clamp(newFocusedThumbIdx, 0, values.size() - 1);
     if (focusedThumbIdx == oldFocusedThumbIdx) {
       // Move focus to next or previous view.
       return false;
@@ -1914,17 +1924,43 @@ private boolean moveFocus(int direction) {
     }
   }
 
+  /**
+   * Attempts to move focus to the <i>left or right</i> of currently focused thumb
+   * and returns whether the focused thumb changed.
+   * If focused thumb didn't change, we're at the view boundary for specified {@code direction}
+   * and focus may be moved to next or previous view instead.
+   * @see #moveFocus(int)
+   */
+  private boolean moveFocusInAbsoluteDirection(int direction) {
+    if (isRtl()) {
+      // Prevent integer overflow.
+      direction = direction == Integer.MIN_VALUE ? Integer.MAX_VALUE : -direction;
+    }
+    return moveFocus(direction);
+  }
+
   private Float calculateIncrementForKey(int keyCode) {
     // If this is a long press, increase the increment so it will only take around 20 steps.
     // Otherwise choose the smallest valid increment.
     float increment = isLongPress ? calculateStepIncrement(20) : calculateStepIncrement();
     switch (keyCode) {
       case KeyEvent.KEYCODE_DPAD_LEFT:
+        if (isRtl()) {
+          return increment;
+        } else {
+          return -increment;
+        }
+      case KeyEvent.KEYCODE_DPAD_RIGHT:
+        if (isRtl()) {
+          return -increment;
+        } else {
+          return increment;
+        }
       case KeyEvent.KEYCODE_MINUS:
         return -increment;
-      case KeyEvent.KEYCODE_DPAD_RIGHT:
-      case KeyEvent.KEYCODE_PLUS:
       case KeyEvent.KEYCODE_EQUALS:
+        // Numpad Plus == Shift + Equals, at least in AVD, so fall through.
+      case KeyEvent.KEYCODE_PLUS:
         return increment;
       default:
         return null;
@@ -1969,13 +2005,17 @@ protected void onFocusChanged(
   private void focusThumbOnFocusGained(int direction) {
     switch (direction) {
       case FOCUS_BACKWARD:
-      case FOCUS_LEFT:
         moveFocus(Integer.MAX_VALUE);
         break;
+      case FOCUS_LEFT:
+        moveFocusInAbsoluteDirection(Integer.MAX_VALUE);
+        break;
       case FOCUS_FORWARD:
-      case FOCUS_RIGHT:
         moveFocus(Integer.MIN_VALUE);
         break;
+      case FOCUS_RIGHT:
+        moveFocusInAbsoluteDirection(Integer.MIN_VALUE);
+        break;
       case FOCUS_UP:
       case FOCUS_DOWN:
       default:
@@ -2229,7 +2269,7 @@ protected boolean onPerformActionForVirtualView(
             }
 
             // Swap the increment if we're in RTL.
-            if (ViewCompat.getLayoutDirection(slider) == ViewCompat.LAYOUT_DIRECTION_RTL) {
+            if (slider.isRtl()) {
               increment = -increment;
             }
 

lib/javatests/com/google/android/material/slider/AndroidManifest.xml

@@ -21,5 +21,7 @@
   <uses-sdk
     tools:overrideLibrary="androidx.test.core"/>
 
-  <application/>
+  <application
+    android:supportsRtl="true"
+    />
 </manifest>

lib/javatests/com/google/android/material/slider/RtlTestUtils.java

@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.material.slider;
+
+import static android.os.Build.VERSION.SDK_INT;
+import static com.google.common.truth.Truth.assertThat;
+
+import android.app.Application;
+import android.content.pm.ApplicationInfo;
+import android.content.res.Configuration;
+import android.content.res.Resources;
+import android.os.Build.VERSION_CODES;
+import android.util.DisplayMetrics;
+import androidx.annotation.RequiresApi;
+import androidx.test.core.app.ApplicationProvider;
+import java.util.Locale;
+
+public final class RtlTestUtils {
+
+  public static void checkAppSupportsRtl() {
+    Application application = ApplicationProvider.getApplicationContext();
+    ApplicationInfo info = application.getApplicationInfo();
+    boolean supportsRtl = (info.flags & ApplicationInfo.FLAG_SUPPORTS_RTL) != 0;
+    assertThat(supportsRtl).isTrue();
+    assertThat(info.targetSdkVersion).isGreaterThan(16);
+  }
+
+  public static void checkPlatformSupportsRtl() {
+    assertThat(SDK_INT).isGreaterThan(16);
+  }
+
+  @RequiresApi(api = VERSION_CODES.JELLY_BEAN_MR1)
+  public static void applyRtlPseudoLocale() {
+    setLocale(new Locale("ar", "XB"));
+  }
+
+  /**
+   * @see org.robolectric.RuntimeEnvironment#setQualifiers(String)
+   */
+  @RequiresApi(api = VERSION_CODES.JELLY_BEAN_MR1)
+  @SuppressWarnings("deprecation")
+  private static void setLocale(Locale locale) {
+    Configuration configuration = new Configuration(Resources.getSystem().getConfiguration());
+    DisplayMetrics displayMetrics = new DisplayMetrics();
+    displayMetrics.setTo(Resources.getSystem().getDisplayMetrics());
+
+    configuration.setLocale(locale);
+
+    Resources systemResources = Resources.getSystem();
+    systemResources.updateConfiguration(configuration, displayMetrics);
+    ApplicationProvider
+        .getApplicationContext()
+        .getResources()
+        .updateConfiguration(configuration, displayMetrics);
+  }
+
+  private RtlTestUtils() {
+    throw new AssertionError();
+  }
+}