-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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(reactnative): fix accessibility for component preview (iOS+VoiceOver) #4601
fix(reactnative): fix accessibility for component preview (iOS+VoiceOver) #4601
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4601 +/- ##
==========================================
- Coverage 35.59% 35.58% -0.01%
==========================================
Files 557 557
Lines 6732 6730 -2
Branches 884 883 -1
==========================================
- Hits 2396 2395 -1
Misses 3876 3876
+ Partials 460 459 -1
Continue to review full report at Codecov.
|
Generated by 🚫 dangerJS |
@Gongreg new contributor here (this is just my second PR, along with #4597). Do I need to fix this coverage decrease? I hope I can contribute more with react-native related PRs. But I need to fix a workflow issue first. I wasn't able to properly test my code changes. I run How do you guys do to test your react-native changes? |
Hey @elucaswork, glad to hear that you want to contribute with React Native. Truth to be told whenever I want to install anything in react native I use: And you are correct, the flow is:
Can you update me if this helps? Also if you want any ideas I wrote about them here: https://medium.com/storybookjs/whats-new-in-storybook-4-0-react-native-741c7f481bbb |
@Gongreg awesome! I'm going to test this flow! Do I need to fix this coverage decrease and make the CI pass? If so, do you have a suggestion on how to make it?
Nice work man! Thanks for doing it. I want to start making some small improvements/fixes on it, but hopefully, I'll be able to make major contributions. Example of improvement: |
@elucaswork Great catch with navigation. I suggest adding keyboard avoiding view. In the long run we will use webview for navigation, but it will still require keyboard avoiding view. Regarding test coverage, don't mind it yet, we still don't test storybook properly. |
Issue: N/A
What I did
On React-Native,
<StoryView>
is wrapped by a<TouchableOpacity>
, and by default touchable elements on RN has anaccessible={true}
(docs).This causes this problem: VoiceOver won't focus on children elements when the parent has
accessible={ true }
, which means it's not possible to fully test the accessibility of components inside Storybook.So the easiest solution for it is settting
accessible={false}
for the<TouchableOpacity>
How to test
Is this testable with Jest or Chromatic screenshots? N
Does this need a new example in the kitchen sink apps? N
Does this need an update to the documentation? N
If your answer is yes to any of these, please make sure to include it in your PR.
For maintainers only: Please tag your pull request with at least one of the following:
["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]