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

RN: Add accessibility labels to OnDeviceUI #1780

Merged
merged 2 commits into from
Sep 6, 2017

Conversation

peritus
Copy link
Contributor

@peritus peritus commented Aug 31, 2017

This adds two added properties, accessibilityLabel (for Android) and testID (for iOS) to the OnDeviceUI controls of the component.

Main motivation for this is so that Appium can inspect and automate the UI rendered by Storybook (for automation testing, buy mainly screenshotting).

The values are just two strings that uniquely identify the component (similar to id in DOM land). The strings I'm proposing here kind of follow a namespaced internal approach that has proven useful in the future. Also happy to change the strings to something else, if need be.

Main motivation for this is so that Appium[1] can inspect and automate the UI rendered by Storybook.

The two added properties are

* accessibilityLabel[2] for Android
* testID[3] for iOS

[1] http://appium.io/
[2] https://facebook.github.io/react-native/docs/view.html#accessibilitylabel
[3] https://facebook.github.io/react-native/docs/view.html#testid
Copy link
Contributor

@rmevans9 rmevans9 left a comment

Choose a reason for hiding this comment

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

LGTM. I really wish react-native would make testid work on both...

@Hypnosphi Hypnosphi changed the base branch from master to release/3.3 September 6, 2017 09:43
@codecov
Copy link

codecov bot commented Sep 6, 2017

Codecov Report

Merging #1780 into release/3.3 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release/3.3    #1780   +/-   ##
============================================
  Coverage        23.14%   23.14%           
============================================
  Files              253      253           
  Lines             5756     5756           
  Branches           695      689    -6     
============================================
  Hits              1332     1332           
+ Misses            3930     3919   -11     
- Partials           494      505   +11
Impacted Files Coverage Δ
...tive/src/preview/components/StoryListView/index.js 0% <ø> (ø) ⬆️
...-native/src/preview/components/OnDeviceUI/index.js 0% <ø> (ø) ⬆️
lib/codemod/src/transforms/update-addon-info.js 51.16% <0%> (ø) ⬆️
addons/knobs/src/components/types/Boolean.js 11.62% <0%> (ø) ⬆️
.../src/manager/containers/CommentsPanel/dataStore.js 34.78% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_routing.js 26.74% <0%> (ø) ⬆️
lib/ui/src/modules/ui/libs/hierarchy.js 50.74% <0%> (ø) ⬆️
addons/storyshots/src/storybook-channel-mock.js 22.22% <0%> (ø) ⬆️
addons/info/src/components/markdown/code.js 24.13% <0%> (ø) ⬆️
addons/info/src/components/PropVal.js 43.15% <0%> (ø) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7610652...4a224d7. Read the comment docs.

@Hypnosphi Hypnosphi self-assigned this Sep 6, 2017
@Hypnosphi Hypnosphi merged commit db01fac into storybookjs:release/3.3 Sep 6, 2017
@hellogerard
Copy link

Been using these new IDs for automated screenshot testing of my Storybook stories. Works great. I recently ran into a limitation: if the list item is far off the screen, then the ID cannot be found. I suppose this is some kind of lazy-rendering optimization in React Native, although I thought only FlatList's lazily rendered, and not ListView's. I could in theory scroll the story list, which would be a pain, but I would still need a test ID on the ListView. I wonder if anyone has any ideas....?

@peritus
Copy link
Contributor Author

peritus commented Dec 14, 2017

@hellogerard yes, ran in to the same problem.

didn't follow through on my automation efforts yet, but if I do I will probably employ the strategy you mentioned:

WHILE NOT id_found OR end_of_list_reached:
  scroll_list_by_half_a_screen()
  id_found = search_for_id()

...

Another option would be to add a search field that allows (also automated scripts) to search for components ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants