Fix/30842 - Add accessibilityState prop in slider #31145

sladyn98 · 2021-03-12T14:00:05Z

Summary

Accessibility service does not announce "selected" on accessibilityState = {selected: true} of the Button Component.
Issue link - #30956

Changelog

[General] [Added] - Add accessibilityState prop to Slider component

Test Plan

Need to create a slider example and tested it on VoiceOver and Talkback

facebook-github-bot · 2021-03-12T14:00:10Z

Hi @sladyn98!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

sladyn98 · 2021-03-12T14:02:57Z

Do I need to add a test to this PR and I would like to know how I can manually test this PR thanks using Talkback. Thanks

analysis-bot · 2021-03-12T14:24:14Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: e6dc371

analysis-bot · 2021-03-12T14:33:57Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,910,251	+57,094
android	hermes	armeabi-v7a	8,409,085	+34,435
android	hermes	x86	9,400,769	+90,692
android	hermes	x86_64	9,344,720	+91,731
android	jsc	arm64-v8a	10,643,518	+59,430
android	jsc	armeabi-v7a	10,125,466	+36,779
android	jsc	x86	10,695,543	+93,050
android	jsc	x86_64	11,279,764	+94,079

Base commit: e6dc371

facebook-github-bot · 2021-03-12T14:42:26Z

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

fabOnReact · 2021-03-16T14:46:23Z

Awesome! Do you think that we should include a test in the rn-tester Accessibility Example? I tought about adding that in my pr as I saw other developers doing the same. 🙏 ☮️

CLICK TO OPEN TESTS RESULTS - SCREENSHOT

amarlette · 2021-03-17T04:49:33Z

Thank you @sladyn98 for opening this PR! RN-tester is preferred for testing https://github.com/facebook/react-native/tree/master/packages/rn-tester, it makes it easier to review PRs and is helpful for reviewers.

@fabriziobertoglio1987 Yes please include examples!

lunaleaps

Thanks for putting this together @sladyn98! As was mentioned in the comments, can we add an example to RNTester with the accessibility examples? Also, were you able to test this on both iOS and Android? I can help test either platform if you don't have access to one

lunaleaps · 2021-03-31T17:54:33Z

Libraries/Components/Slider/Slider.js

@@ -197,6 +204,7 @@ const Slider = (
  forwardedRef?: ?React.Ref<typeof SliderNativeComponent>,
 ) => {
  const style = StyleSheet.compose(styles.slider, props.style);
+  const accessibilityState = props.accessibilityState || {};


Does this need to be an empty object if no props.accessibilityState? It seems like SliderNativeComponent allows for null value for accessibilityState prop. In this approach you'd be a creating a non-used new object every time.

Yeah I was wondering too, so should we just omit the empty object since props.accessibilityState would return null if its empty

Yea we can directly pass the prop to the native component.

lunaleaps

Requesting changes as from my comment

lunaleaps · 2021-04-06T20:58:41Z

@sladyn98 Let me know if you have any questions or change in plans to working on this

sladyn98 · 2021-04-07T10:53:47Z

packages/rn-tester/js/examples/Accessibility/AccessibilityExample.js

+            }}>
+            <Text>Slider</Text>
+          </View>
+        </RNTesterBlock>


This is my first time trying to add a test/example, woud I need to set the accessibilityState={{disabled: true}} as initially ? and how would i set it later on like when the slider is used should I alert that the state has changed

I think the original issue is that the Slider component does not announce its selection on focus. So you'd want to render a Slider component here .. I'm not sure if there's a callback for an onFocus here. But at least with Talkback/VoiceOver on we'd want to confirm that the Slider state is being announced. Not sure the best way to illustrate that in the example

Oh maybe I can ask @fabriziobertoglio1987 to assist us here

@sladyn98 Here are the docs for the Slider component 🙏 https://reactnative.dev/docs/next/slider

I added an example here, how do I get i set the accessibility State here ? like it does not have a property pertaining to that

@sladyn98 I'm not sure I understand your question. I think you want to render something like:

<Slider maximumValue={maximumValue} minimumValue={minimumValue} onSlidingComplete={onSlidingComplete} value={value} accessibilityState={{disabled: true}} // or whatever accessibilityState you'd want to pass />

and then verify with TalkBack that the slider is respecting the accessibilityState passed. The one I'm most familiar with is disability but there are others: https://reactnative.dev/docs/accessibility#accessibilitystate

Some of them may not make sense on the Slider component like busy/expanded but maybe you could experiment and add a comment about what is expected?

Whoops sorry about the delay, forgot to hit submit!

lunaleaps · 2021-04-26T22:26:02Z

@sladyn98 Are you planning to flesh out the example? If not, we can take this in as is and I can help flesh out the example for Slider. Let me know what you want to do!

facebook-github-bot · 2021-05-10T22:38:19Z

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2021-05-21T20:36:13Z

@lunaleaps merged this pull request in 35dd861.

sladyn98 · 2021-05-26T12:27:40Z

@lunaleaps Yeah I am not sure how to add the example so if you could that would be great

fix: 30842 - add accessibilityState prop in slider

cddc86f

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 12, 2021

amarlette added the Accessibility label Mar 15, 2021

amarlette requested a review from kacieb March 15, 2021 23:14

amarlette linked an issue Mar 15, 2021 that may be closed by this pull request

Selected State does not annonce when Slider Component selected #30956

Closed

kacieb assigned lunaleaps Mar 29, 2021

kacieb requested a review from lunaleaps March 29, 2021 16:20

lunaleaps reviewed Mar 31, 2021

View reviewed changes

lunaleaps requested changes Mar 31, 2021

View reviewed changes

lunaleaps mentioned this pull request Mar 31, 2021

Selected State does not annonce when Slider Component selected #30956

Closed

Add rn-tester example

170d6ca

sladyn98 commented Apr 7, 2021

View reviewed changes

Add example

0d51104

facebook-github-bot closed this in 35dd861 May 21, 2021

facebook-github-bot added the Merged This PR has been merged. label May 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix/30842 - Add accessibilityState prop in slider #31145

Fix/30842 - Add accessibilityState prop in slider #31145

sladyn98 commented Mar 12, 2021

facebook-github-bot commented Mar 12, 2021

sladyn98 commented Mar 12, 2021

analysis-bot commented Mar 12, 2021 •

edited

Loading

analysis-bot commented Mar 12, 2021 •

edited

Loading

facebook-github-bot commented Mar 12, 2021

fabOnReact commented Mar 16, 2021 •

edited

Loading

amarlette commented Mar 17, 2021

lunaleaps left a comment

lunaleaps Mar 31, 2021

sladyn98 Apr 7, 2021

lunaleaps Apr 7, 2021

lunaleaps left a comment

lunaleaps commented Apr 6, 2021

sladyn98 Apr 7, 2021

lunaleaps Apr 7, 2021

sladyn98 Apr 9, 2021

fabOnReact Apr 9, 2021

sladyn98 Apr 15, 2021

lunaleaps Apr 19, 2021

lunaleaps Apr 20, 2021

lunaleaps commented Apr 26, 2021

facebook-github-bot commented May 10, 2021

facebook-github-bot commented May 21, 2021

sladyn98 commented May 26, 2021

Fix/30842 - Add accessibilityState prop in slider #31145

Fix/30842 - Add accessibilityState prop in slider #31145

Conversation

sladyn98 commented Mar 12, 2021

Summary

Changelog

Test Plan

facebook-github-bot commented Mar 12, 2021

Action Required

Process

sladyn98 commented Mar 12, 2021

analysis-bot commented Mar 12, 2021 • edited Loading

analysis-bot commented Mar 12, 2021 • edited Loading

facebook-github-bot commented Mar 12, 2021

fabOnReact commented Mar 16, 2021 • edited Loading

amarlette commented Mar 17, 2021

lunaleaps left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunaleaps left a comment

Choose a reason for hiding this comment

lunaleaps commented Apr 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lunaleaps commented Apr 26, 2021

facebook-github-bot commented May 10, 2021

facebook-github-bot commented May 21, 2021

sladyn98 commented May 26, 2021

analysis-bot commented Mar 12, 2021 •

edited

Loading

analysis-bot commented Mar 12, 2021 •

edited

Loading

fabOnReact commented Mar 16, 2021 •

edited

Loading