Skip to content

Commit

Permalink
Fix: Conflicting states for accessibility and non-accessibility users…
Browse files Browse the repository at this point in the history
… (#354)

* adding accessibilityState.disabled prop to Slider.js

* removing java logic to trigger accessibility disabled announcement

* jest tests accessiblityState.disabled and disabled prop

* update jest snapshots

* adding invariant check on disabled prop type

Flow check on master seems to not work
fabOnReact/react-native-notes#2 (comment)

As an alternative I implemented an invariant check.
https://github.com/fabriziobertoglio1987/react-native/blob/76a2cf3569571b943b4bcc6867e069338ff88f1f/Libraries/Lists/VirtualizedList.js#L1240-L1245
https://github.com/zertosh/invariant

* adding Slider disabled example

* trigger invariant error when props.disabled is null

* remove check on prop type commit 16d321b

I will try to fix .flowconfig and enable flow type checking for Slider props

* improve test cases descriptions

* add LogBox (console.warn) message to notify developer conflicting accessibility disabled state

* update LogBox warning text

* remove LogBox warning

* jest fix obsolete snapshots

* Add test for reverted disabled/enabled case

There's already a test, where disabled={true} is checked against
accessibilityState={{disabled: false}},
but to double check all the combinations this commit adds new test
that acts as the inverted scenario to make sure that conflicts are
avoided both directions.

* avoid conflicting disabled states

It would be better to show that both these properties can be used separately without fear of another.
callstack/react-native-slider#354 (comment)

Co-authored-by: BartoszKlonowski <Bartosz.Klonowski@callstack.com>
  • Loading branch information
fabOnReact and BartoszKlonowski committed Feb 28, 2022
1 parent a183fb6 commit 1a46f1e
Show file tree
Hide file tree
Showing 5 changed files with 146 additions and 14 deletions.
14 changes: 14 additions & 0 deletions example/SliderExample.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,4 +223,18 @@ export const examples = [
return <SliderExample value={0.6} vertical />;
},
},
{
title: 'Disabled slider',
platform: 'android',
render(): Element<any> {
return <SliderExample disabled value={0.6} />;
},
},
{
title: 'Slider with accessibilityState disabled',
platform: 'android',
render(): Element<any> {
return <SliderExample disabled value={0.6} />;
},
},
];
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import android.graphics.drawable.BitmapDrawable;
import android.os.Build;
import android.util.AttributeSet;
import android.view.MotionEvent;
import android.view.accessibility.AccessibilityEvent;
import android.view.accessibility.AccessibilityManager;
import androidx.appcompat.widget.AppCompatSeekBar;
Expand Down Expand Up @@ -153,17 +152,6 @@ public void run() {
}
}

@Override
public boolean onTouchEvent(MotionEvent arg0) {
super.onTouchEvent(arg0);

if (arg0.getActionMasked() == MotionEvent.ACTION_DOWN && this.isEnabled() == false) {
announceForAccessibility("slider disabled");
}
// Returns: True if the view handled the hover event
return true;
}

public void setupAccessibility(int index) {
if (mAccessibilityUnits != null && mAccessibilityIncrements != null && mAccessibilityIncrements.size() - 1 == (int)mMaxValue) {
String sliderValue = mAccessibilityIncrements.get(index);
Expand Down
15 changes: 13 additions & 2 deletions src/js/Slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,16 @@ const SliderComponent = (
}
: null;

const _disabled =
typeof props.disabled === 'boolean'
? props.disabled
: props.accessibilityState?.disabled === true;

const _accessibilityState =
typeof props.disabled === 'boolean'
? {...props.accessibilityState, disabled: props.disabled}
: props.accessibilityState;

const onChangeEvent = onValueChangeEvent;
const onSlidingStartEvent = onSlidingStart
? (event: Event) => {
Expand All @@ -289,9 +299,11 @@ const SliderComponent = (
onRNCSliderSlidingStart={onSlidingStartEvent}
onRNCSliderSlidingComplete={onSlidingCompleteEvent}
onRNCSliderValueChange={onValueChangeEvent}
enabled={!props.disabled}
enabled={!_disabled}
disabled={_disabled}
onStartShouldSetResponder={() => true}
onResponderTerminationRequest={() => false}
accessibilityState={_accessibilityState}
/>
);
};
Expand All @@ -303,7 +315,6 @@ const SliderWithRef = React.forwardRef(SliderComponent);
* and run Flow. */

SliderWithRef.defaultProps = {
disabled: false,
value: 0,
minimumValue: 0,
maximumValue: 1,
Expand Down
24 changes: 24 additions & 0 deletions src/js/__tests__/Slider.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,30 @@ describe('<Slider />', () => {
expect(tree).toMatchSnapshot();
});

it('accessibilityState disabled sets disabled={true}', () => {
const tree = renderer
.create(<Slider accessibilityState={{disabled: true}} />)
.toJSON();

expect(tree).toMatchSnapshot();
});

it('disabled prop overrides accessibilityState.disabled', () => {
const tree = renderer
.create(<Slider disabled accessibilityState={{disabled: false}} />)
.toJSON();

expect(tree).toMatchSnapshot();
});

it('disabled prop overrides accessibilityState.enabled', () => {
const tree = renderer
.create(<Slider disabled={false} accessibilityState={{disabled: true}} />)
.toJSON();

expect(tree).toMatchSnapshot();
});

it('renders a slider with custom props', () => {
const tree = renderer
.create(
Expand Down
95 changes: 95 additions & 0 deletions src/js/__tests__/__snapshots__/Slider.test.js.snap
Original file line number Diff line number Diff line change
@@ -1,5 +1,65 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`<Slider /> accessibilityState disabled sets disabled={true} 1`] = `
<RNCSlider
accessibilityState={
Object {
"disabled": true,
}
}
disabled={true}
enabled={false}
inverted={false}
maximumValue={1}
minimumValue={0}
onChange={null}
onRNCSliderSlidingComplete={null}
onRNCSliderSlidingStart={null}
onRNCSliderValueChange={null}
onResponderTerminationRequest={[Function]}
onStartShouldSetResponder={[Function]}
step={0}
style={
Object {
"height": 40,
}
}
tapToSeek={false}
thumbImage={null}
value={0}
/>
`;

exports[`<Slider /> disabled prop overrides accessibilityState.disabled 1`] = `
<RNCSlider
accessibilityState={
Object {
"disabled": true,
}
}
disabled={true}
enabled={false}
inverted={false}
maximumValue={1}
minimumValue={0}
onChange={null}
onRNCSliderSlidingComplete={null}
onRNCSliderSlidingStart={null}
onRNCSliderValueChange={null}
onResponderTerminationRequest={[Function]}
onStartShouldSetResponder={[Function]}
step={0}
style={
Object {
"height": 40,
}
}
tapToSeek={false}
thumbImage={null}
value={0}
/>
`;

exports[`<Slider /> renders a slider with custom props 1`] = `
<RNCSlider
disabled={false}
Expand Down Expand Up @@ -30,6 +90,11 @@ exports[`<Slider /> renders a slider with custom props 1`] = `

exports[`<Slider /> renders disabled slider 1`] = `
<RNCSlider
accessibilityState={
Object {
"disabled": true,
}
}
disabled={true}
enabled={false}
inverted={false}
Expand Down Expand Up @@ -77,3 +142,33 @@ exports[`<Slider /> renders enabled slider 1`] = `
value={0}
/>
`;

exports[`<Slider /> disabled prop overrides accessibilityState.enabled 1`] = `
<RNCSlider
accessibilityState={
Object {
"disabled": false,
}
}
disabled={false}
enabled={true}
inverted={false}
maximumValue={1}
minimumValue={0}
onChange={null}
onRNCSliderSlidingComplete={null}
onRNCSliderSlidingStart={null}
onRNCSliderValueChange={null}
onResponderTerminationRequest={[Function]}
onStartShouldSetResponder={[Function]}
step={0}
style={
Object {
"height": 40,
}
}
tapToSeek={false}
thumbImage={null}
value={0}
/>
`;

0 comments on commit 1a46f1e

Please sign in to comment.