Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix keyboard progress increment getting stuck at 6 #572

Merged

Conversation

vafada
Copy link
Contributor

@vafada vafada commented Jan 31, 2024

Summary:

This fixes #155

see #155 (comment) for details of the issue

The gist of it, on initial load ReactSlider calls

setMax(128) due to DEFAULT_TOTAL_STEPS = 128

which calls the AbsSeekBar.java's setMax()

https://cs.android.com/android/platform/superproject/main/+/main:frameworks/base/core/java/android/widget/AbsSeekBar.java;l=586-595;drc=c8c729556d69aa27fff24e5412140563fc1527af

and internally updates mKeyProgressIncrement to 6

that causes keyboard input to jump 6 steps:

https://android.googlesource.com/platform/frameworks/base/+/711efaa0297f845c60a1de0acce680493f90bf02/core/java/android/widget/AbsSeekBar.java#1053

after setting ReactSlider.setMaxValue, if max is not high enough, the keyProgressIncrement will still get stuck at 6

due to this logic in AbsSeekBar:

https://android.googlesource.com/platform/frameworks/base/+/711efaa0297f845c60a1de0acce680493f90bf02/core/java/android/widget/AbsSeekBar.java#590

Test Plan:

Render a simple Slider

import React, {useState} from 'react';
import {
  SafeAreaView,
  StyleSheet,
  Text,
  View,
} from 'react-native';

import Slider from '@react-native-community/slider';

const App = () => {
  const [value, setValue] = useState(10);


  return (
    <SafeAreaView style={styles.homeScreenContainer}>
      <View>
        <Text testID="testTextId" style={styles.title}>
          Value: {value}
        </Text>
        <Slider
          step={1}
          maximumValue={30}
            minimumValue={0}
            style={styles.slider}
            value={value}
            thumbTintColor={pageViewPositionSlider.thumbColor}
            maximumTrackTintColor={pageViewPositionSlider.trackColor}
            minimumTrackTintColor={pageViewPositionSlider.trackColor}
          onValueChange={setValue}
        />
      </View>
    </SafeAreaView>
  );
};

export default App;

const pageViewPositionSlider = {
  trackColor: '#ABABAB',
  thumbColor: '#1411AB',
  style: {
    width: '100%',
  },
};

const styles = StyleSheet.create({
  slider: {
    width: '100%',
  },
  pagerViewContainer: {
    flex: 1,
  },
  homeScreenContainer: {
    flex: 1,
  },
  scrollView: {
    backgroundColor: '#F5FCFF',
  },
  container: {
    justifyContent: 'center',
    alignItems: 'center',
    paddingVertical: 20,
  },
  title: {
    fontSize: 30,
    color: pageViewPositionSlider.thumbColor,
    textAlign: 'center',
    width: '100%',
    marginVertical: 20,
  },
  instructions: {
    textAlign: 'center',
    color: '#333333',
    marginBottom: 5,
    fontSize: 20,
  },
  sliderWidget: {
    marginVertical: 30,
  },
});

use the keyboard to navigate to the slider (tabs on android or use a virtual keyboard)

Verify that LEFT and RIGHT arrow moves the thumb according to the step prop

@BartoszKlonowski BartoszKlonowski added the platform: Android Issue related to Android platform label Feb 2, 2024
Copy link
Member

@BartoszKlonowski BartoszKlonowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks decent and works like a charm!
I would keep the explanation of the fix in the commit message or PR description (which is well written - thank you!) instead of putting it in the code, as after some time and multiple fixes it would get noisy with all the comments.

@BartoszKlonowski BartoszKlonowski merged commit 87d28cb into callstack:main Feb 23, 2024
7 of 8 checks passed
@vafada vafada deleted the fix-android-keyboard-step-jump branch February 23, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Android Issue related to Android platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Slider does not respect the step value on when using with keyboard
2 participants