From 868fbeaa00219219d72bc11f4756647f42984fe7 Mon Sep 17 00:00:00 2001 From: Rene Weber Date: Wed, 9 Nov 2016 19:55:45 -0800 Subject: [PATCH] Make sure to re-calculate step if not explicitly set Summary: This causes the step to be re-calculated on every update of min, max and step value, to use the most up to date values for the calculation, except if step is explicitly set to a non-zero value by the user. Fixes #10253 **Test plan (required)** 1. Create example app 2. Create a view with a slider that has a `value`, `minimumValue` and `maximumValue` set, but no step value (or step value set to 0). For example: ``` ``` 3. See slider working as expected Closes https://github.com/facebook/react-native/pull/10343 Differential Revision: D4142646 Pulled By: hramos fbshipit-source-id: a0df87bbdbbd4b2a291d89f5579f73f517a33dfc --- .../react/views/slider/ReactSlider.java | 13 ++- .../test/java/com/facebook/react/views/BUCK | 1 + .../views/slider/ReactSliderPropertyTest.java | 105 ++++++++++++++++++ 3 files changed, 115 insertions(+), 4 deletions(-) create mode 100644 ReactAndroid/src/test/java/com/facebook/react/views/slider/ReactSliderPropertyTest.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/slider/ReactSlider.java b/ReactAndroid/src/main/java/com/facebook/react/views/slider/ReactSlider.java index 5c6710d2f..666d9d146 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/slider/ReactSlider.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/slider/ReactSlider.java @@ -30,7 +30,7 @@ public class ReactSlider extends SeekBar { /** * If step is 0 (unset) we default to this total number of steps. * Don't use 100 which leads to rounding errors (0.200000000001). - */ + */ private static int DEFAULT_TOTAL_STEPS = 128; /** @@ -50,6 +50,7 @@ public class ReactSlider extends SeekBar { * If zero it's determined automatically. */ private double mStep = 0; + private double mStepCalculated = 0; public ReactSlider(Context context, @Nullable AttributeSet attrs, int style) { super(context, attrs, style); @@ -83,7 +84,7 @@ public class ReactSlider extends SeekBar { if (seekBarProgress == getMax()) { return mMaxValue; } - return seekBarProgress * mStep + mMinValue; + return seekBarProgress * getStepValue() + mMinValue; } /** @@ -91,7 +92,7 @@ public class ReactSlider extends SeekBar { */ private void updateAll() { if (mStep == 0) { - mStep = (mMaxValue - mMinValue) / (double) DEFAULT_TOTAL_STEPS; + mStepCalculated = (mMaxValue - mMinValue) / (double) DEFAULT_TOTAL_STEPS; } setMax(getTotalSteps()); updateValue(); @@ -106,6 +107,10 @@ public class ReactSlider extends SeekBar { } private int getTotalSteps() { - return (int) Math.ceil((mMaxValue - mMinValue) / mStep); + return (int) Math.ceil((mMaxValue - mMinValue) / getStepValue()); + } + + private double getStepValue() { + return mStep > 0 ? mStep : mStepCalculated; } } diff --git a/ReactAndroid/src/test/java/com/facebook/react/views/BUCK b/ReactAndroid/src/test/java/com/facebook/react/views/BUCK index 491648c57..9f4f1ca2c 100644 --- a/ReactAndroid/src/test/java/com/facebook/react/views/BUCK +++ b/ReactAndroid/src/test/java/com/facebook/react/views/BUCK @@ -24,6 +24,7 @@ robolectric3_test( react_native_target('java/com/facebook/react/uimanager/annotations:annotations'), react_native_target('java/com/facebook/react/uimanager:uimanager'), react_native_target('java/com/facebook/react/views/image:image'), + react_native_target('java/com/facebook/react/views/slider:slider'), react_native_target('java/com/facebook/react/views/text:text'), react_native_target('java/com/facebook/react/views/textinput:textinput'), react_native_target('java/com/facebook/react/views/view:view'), diff --git a/ReactAndroid/src/test/java/com/facebook/react/views/slider/ReactSliderPropertyTest.java b/ReactAndroid/src/test/java/com/facebook/react/views/slider/ReactSliderPropertyTest.java new file mode 100644 index 000000000..7cb756121 --- /dev/null +++ b/ReactAndroid/src/test/java/com/facebook/react/views/slider/ReactSliderPropertyTest.java @@ -0,0 +1,105 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.react.views.slider; + +import android.widget.SeekBar; +import com.facebook.react.bridge.CatalystInstance; +import com.facebook.react.bridge.JavaOnlyMap; +import com.facebook.react.bridge.ReactApplicationContext; +import com.facebook.react.bridge.ReactTestHelper; +import com.facebook.react.uimanager.ReactStylesDiffMap; +import com.facebook.react.uimanager.ThemedReactContext; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.modules.junit4.rule.PowerMockRule; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.RuntimeEnvironment; + +import static org.fest.assertions.api.Assertions.assertThat; + +/** + * Verify {@link SeekBar} view property being applied properly by {@link ReactSliderManager} + */ +@RunWith(RobolectricTestRunner.class) +@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"}) +public class ReactSliderPropertyTest { + + @Rule + public PowerMockRule rule = new PowerMockRule(); + + private ThemedReactContext mThemedContext; + private ReactSliderManager mManager; + + @Before + public void setup() { + ReactApplicationContext mContext = new ReactApplicationContext(RuntimeEnvironment.application); + CatalystInstance mCatalystInstanceMock = ReactTestHelper.createMockCatalystInstance(); + mContext.initializeWithInstance(mCatalystInstanceMock); + mThemedContext = new ThemedReactContext(mContext, mContext); + mManager = new ReactSliderManager(); + } + + public ReactStylesDiffMap buildStyles(Object... keysAndValues) { + return new ReactStylesDiffMap(JavaOnlyMap.of(keysAndValues)); + } + + @Test + public void testValueWithMaxValue() { + ReactSlider view = mManager.createViewInstance(mThemedContext); + + mManager.updateProperties(view, buildStyles("maximumValue", 10.0)); + mManager.updateProperties(view, buildStyles("value", 5.5)); + assertThat(view.getProgress()).isEqualTo(70); + } + + @Test + public void testValueWithMaxValueSetBeforeMinValue() { + ReactSlider view = mManager.createViewInstance(mThemedContext); + + mManager.updateProperties(view, buildStyles("maximumValue", 10.0)); + mManager.updateProperties(view, buildStyles("minimumValue", 5.0)); + mManager.updateProperties(view, buildStyles("value", 5.5)); + assertThat(view.getProgress()).isEqualTo(13); + } + + @Test + public void testValueWithMinValueSetBeforeMaxValue() { + ReactSlider view = mManager.createViewInstance(mThemedContext); + + mManager.updateProperties(view, buildStyles("minimumValue", 5.0)); + mManager.updateProperties(view, buildStyles("maximumValue", 10.0)); + mManager.updateProperties(view, buildStyles("value", 5.5)); + assertThat(view.getProgress()).isEqualTo(13); + } + + @Test + public void testValueWithMaxValueAndStep() { + ReactSlider view = mManager.createViewInstance(mThemedContext); + + mManager.updateProperties(view, buildStyles("maximumValue", 10.0)); + mManager.updateProperties(view, buildStyles("step", 3.0)); + mManager.updateProperties(view, buildStyles("value", 5.5)); + assertThat(view.getProgress()).isEqualTo(2); + } + + @Test + public void testValueWithMaxValueAndMinValueAndStep() { + ReactSlider view = mManager.createViewInstance(mThemedContext); + + mManager.updateProperties(view, buildStyles("maximumValue", 10.0)); + mManager.updateProperties(view, buildStyles("minimumValue", 5.0)); + mManager.updateProperties(view, buildStyles("step", 3.0)); + mManager.updateProperties(view, buildStyles("value", 10.0)); + assertThat(view.getProgress()).isEqualTo(2); + } +}