fix: android native variant min/max scrolling issues (#286)

* fix: native android variant min/max dates scrolling issues

* add prop check

* keep track of animating state

* update github actions

* temp test

* change work dir

* restore compile sdk

Author: henninghall

.github/workflows/android-detox.yml

@@ -9,8 +9,8 @@ on:
       - master
 
 jobs:
-  unit_tests:
-    name: Unit tests
+  javascript_unit_tests:
+    name: Unit tests - javascript
     runs-on: macos-latest
     timeout-minutes: 5
 
@@ -31,6 +31,26 @@ jobs:
         run: |
           yarn test
 
+  java_unit_tests:
+    name: Unit tests - java
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+      - name: Set up JDK 1.8
+        uses: actions/setup-java@v1
+        with:
+          java-version: 1.8
+
+      - name: Install npm dependencies
+        working-directory: ./examples/detox
+        run: |
+          yarn install --frozen-lockfile
+
+      - name: Run unit tests
+        working-directory: ./examples/detox/android
+        run: ./gradlew testDebugUnitTest
+
   end_to_end_tests:
     name: End to end tests
     runs-on: macos-latest

android/src/main/java/com/henninghall/date_picker/Utils.java

@@ -85,4 +85,17 @@ public static WheelType patternCharToWheelType(char patternChar) throws Exceptio
             default: throw new Exception("Invalid pattern char: " + patternChar);
         }
     }
+
+
+    public static int getShortestScrollOption(int from, int to, final int maxValue, boolean isWrapping) {
+        int size = maxValue + 1;
+        int option1 = to - from;
+        int option2 = option1 > 0 ? option1 - size : option1 + size;
+        if (isWrapping) {
+            return Math.abs(option1) < Math.abs(option2) ? option1 : option2;
+        }
+        if (from + option1 > maxValue) return option2;
+        if (from + option1 < 0) return option2;
+        return option1;
+    }
 }

android/src/main/java/com/henninghall/date_picker/pickers/AndroidNative.java

@@ -9,6 +9,8 @@
 import android.widget.EditText;
 import android.widget.NumberPicker;
 
+import com.henninghall.date_picker.Utils;
+
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -21,6 +23,7 @@ public class AndroidNative extends NumberPicker implements Picker {
     private Picker.OnValueChangeListener onValueChangedListener;
     private int state = SCROLL_STATE_IDLE;
     private OnValueChangeListenerInScrolling listenerInScrolling;
+    private boolean isAnimating;
 
     public AndroidNative(Context context) {
         super(context);
@@ -86,47 +89,39 @@ public View getView() {
     public void setDividerHeight(int height) {
         // not supported
     }
-    
+
     @Override
     public void setItemPaddingHorizontal(int padding) {
         // Not needed for this picker
     }
 
     @Override
     public boolean isSpinning() {
-        return state == SCROLL_STATE_FLING;
+        return state == SCROLL_STATE_FLING || isAnimating;
     }
 
     @Override
     public void smoothScrollToValue(final int value) {
         final AndroidNative self = this;
 
+        int currentValue = self.getValue();
+        if (value == currentValue) return;
+        int shortestScrollOption = Utils.getShortestScrollOption(currentValue, value, getMaxValue(), getWrapSelectorWheel());
+        final int moves = Math.abs(shortestScrollOption);
+        int timeBetweenScrollsMs = 100;
+        int willStopScrollingInMs = timeBetweenScrollsMs * moves;
+        isAnimating = true;
         new Handler().postDelayed(new Runnable() {
+            @Override
             public void run() {
-                int currentValue = self.getValue();
-                if (value == currentValue) return;
-                int shortestScrollOption = getShortestScrollOption(currentValue, value);
-                final int moves = Math.abs(shortestScrollOption);
-                for (int i = 0; i < moves; i++) {
-                    // need some delay between each scroll step to make sure it scrolls to correct value
-                    changeValueByOne(shortestScrollOption > 0, i * 100, i == moves - 1);
-                }
+                isAnimating = false;
             }
-            // since the SCROLL_STATE_IDLE event is dispatched before the wheel actually has stopped
-            // an extra delay has to be added before starting to scroll to correct value
-        }, 500);
-    }
+        }, willStopScrollingInMs);
 
-    private int getShortestScrollOption(int currentValue, int value) {
-        final int maxValue = getMaxValue();
-        int option1 = value - currentValue;
-        int option2 = maxValue + 1 - Math.abs(option1);
-        if (getWrapSelectorWheel()) {
-            return Math.abs(option1) < Math.abs(option2) ? option1 : option2;
+        for (int i = 0; i < moves; i++) {
+            // need some delay between each scroll step to make sure it scrolls to correct value
+            changeValueByOne(shortestScrollOption > 0, i * timeBetweenScrollsMs, i == moves - 1);
         }
-        if (currentValue + option1 > maxValue) return option2;
-        if (currentValue + option1 < 0) return option2;
-        return option1;
     }
 
     private void changeValueByOne(final NumberPicker higherPicker, final boolean increment) {
@@ -185,16 +180,16 @@ public void onValueChange(NumberPicker numberPicker, int oldVal, int newVal) {
                 // to send event during scrolling we make sure wheel is still. This particular
                 // case happens when wheel is tapped, not scrolled.
                 if(state == SCROLL_STATE_IDLE) {
-                  sendEventIn500ms();
+                    sendEventIn500ms();
                 }
             }
         });
 
         super.setOnScrollListener(new OnScrollListener() {
             @Override
             public void onScrollStateChange(NumberPicker numberPicker, int nextState) {
-               sendEventIfStopped(nextState);
-               state = nextState;
+                sendEventIfStopped(nextState);
+                state = nextState;
             }
         });
     }

android/src/main/java/com/henninghall/date_picker/ui/WheelChangeListenerImpl.java

@@ -40,7 +40,7 @@ private SimpleDateFormat getDateFormat(){
     public void onChange(Wheel picker) {
         if(wheels.hasSpinningWheel()) return;
 
-        if(!exists()){
+        if(!dateExists()){
             Calendar closestExistingDate = getClosestExistingDateInPast();
             if(closestExistingDate != null) {
                 uiManager.animateToDate(closestExistingDate);
@@ -67,7 +67,7 @@ public void onChange(Wheel picker) {
     }
 
     // Example: Jan 1 returns true, April 31 returns false.
-    private boolean exists(){
+    private boolean dateExists(){
         SimpleDateFormat dateFormat = getDateFormat();
         String toParse = wheels.getDateTimeString();
         try {

examples/detox/android/app/build.gradle

@@ -78,6 +78,7 @@ dependencies {
 
     androidTestImplementation('com.wix:detox:+') { transitive = true }
     androidTestImplementation 'junit:junit:4.12'
+    testImplementation 'junit:junit:4.12'
 }
 
 // Run this once to be able to run the application with BUCK

examples/detox/android/app/src/test/java/ShortestScrollOption.java

@@ -0,0 +1,60 @@
+
+import com.henninghall.date_picker.Utils;
+
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class ShortestScrollOption {
+
+    @Test
+    public void decreaseOne() {
+        assertEquals( -1, Utils.getShortestScrollOption(1, 0, 10, false));
+    }
+
+    @Test
+    public void increaseOne() {
+        assertEquals(1, Utils.getShortestScrollOption(0, 1, 10, false));
+    }
+
+    @Test
+    public void increaseFive() {
+        assertEquals( 5, Utils.getShortestScrollOption(0, 5, 10, false));
+    }
+
+    @Test
+    public void noChange() {
+        assertEquals( 0, Utils.getShortestScrollOption(0, 0, 10, false));
+    }
+
+    @Test
+    public void noWrapping() {
+        assertEquals( 10, Utils.getShortestScrollOption(0, 10, 10, false));
+    }
+
+    @Test
+    public void wrapping() {
+        assertEquals( -1, Utils.getShortestScrollOption(0, 10, 10, true));
+    }
+
+    @Test
+    public void findingClosestByIncreaseNoWrap() {
+        assertEquals( 4, Utils.getShortestScrollOption(0, 4, 9, true));
+    }
+
+    @Test
+    public void findingClosestByIncreaseWrap() {
+        assertEquals( 4, Utils.getShortestScrollOption(6, 0, 9, true));
+    }
+
+    @Test
+    public void findingClosestByDecreaseNoWrap() {
+        assertEquals( -4, Utils.getShortestScrollOption(5, 1, 9, true));
+    }
+
+    @Test
+    public void findingClosestByDecreaseWrap() {
+        assertEquals( -4, Utils.getShortestScrollOption(0, 6, 9, true));
+    }
+
+}

src/propChecker.js

@@ -40,4 +40,10 @@ const modeCheck = new PropCheck(
   "Invalid mode. Valid modes: 'datetime', 'date', 'time'"
 )
 
-const checks = [widthCheck, heightCheck, modeCheck]
+const androidVariantCheck = new PropCheck(
+  props =>
+    props && props.androidVariant && !['nativeAndroid', 'iosClone'].includes(props.androidVariant),
+  "Invalid android variant. Valid modes: 'nativeAndroid', 'iosClone'"
+)
+
+const checks = [widthCheck, heightCheck, modeCheck, androidVariantCheck]