👤🔊 OP Accessibility: Screen Reader Improvements

[sebastianbarry]

Part of issue: e-mission/e-mission-docs#1011
This PR is for the purpose of taking over for @niccolopaganini, #1112

Goals:

• Screen reader reads the entire page when Popups display, instead of only what's on the popup
• Unpronounceable buttons in accessibility inspector
• Profile buttons; some buttons screen-read as doubled the word
• Having screen reader read dropdowns as "dropdown" instead of "button"

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c7e9244) 77.55% compared to head (cf7c860) 77.55%.
Report is 2 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1118   +/-   ##
=======================================
  Coverage   77.55%   77.55%           
=======================================
  Files          28       28           
  Lines        1702     1702           
  Branches      367      367           
=======================================
  Hits         1320     1320           
  Misses        382      382           

Flag	Coverage Δ
unit	77.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[sebastianbarry]

I was able to get Xcode Accessibility Inspector to give me a preview of the screen reader text! It is not as in depth as the Android, so I will probably still test with that + physical android device screen reader, but this is a good thing to know about.

[sebastianbarry]

I also found these discussions that might be helpful for figuring out the popup - screen reader reading what's behind the popup issue:

callstack/react-native-paper#3166

https://stackoverflow.com/questions/72100125/ive-implemented-a-custom-dialog-using-react-native-papers-dialog-component-but

[sebastianbarry]

This is the i18n translation files PR for removing non-character characters e-mission/e-mission-translate#53

[sebastianbarry]

I have completed most of the changes I want to for now. Here is a summary:

#	Change	Description	Emulator Testing Image	Tested on physical test device?
1	Logout button acts as 1 button	Before, the screenreader treated the logout button as a separate text element, and a icon button. Now, they should be paired as one and only one focusable component.		Not tested
2	Unpronounceable metrics card icons on Dashboard	Before, the metrics cards had icons that did not have accessibility text, meaning a screenreader user would not know what these icons represent. Now, their accessibility text is the same as the name of the icon ("details, graph" "user, aggregate").		Not tested
3	Remove non-character characters, like 👇	For the sake of accessibility users, non-character characters are considered bad form (Seren Davies: Death to icon fonts - EpicFEL 2015). Consider screen reader users, where an icon can either read as "unpronounceable", or, at best when labeled, can obscure the meaning of the icon (consider a text box that displays "⭐️ favorites" and screen reads "star favorites"; a screen reader user may think "what is a star favorite, and is it different than non-star favorites??"). Another example of where this can be an issue is in dyslexic-font users, whose fonts may not support non-character characters and they would display as empty boxes []. Any emojis have been removed, as well as in the translated files: e-mission/e-mission-translate#53 The next place to go with this, is implementing a fade-gradient on the scrollable sides (above or below), to give non-screen-reader users a visual que that there are more elements in this list, and then we can remove the text "scroll for more"; see https://github.com/jbccollins/react-gradient-scroll-indicator		Not tested
4	BottomNavigation buttons read double: "Profile Profile"	Before, the screenreader read the BottomNavigation buttons as their text but doubled. I've added an accessibilityLabel field to the BottomNavigation elements, so now ideally the text will not be duplicated as it will be overridden by the accessibilityLabel property. The reason it reads twice is because React Native Paper displays 2 of the exact same text element, and one of them switches from 0 to 1 opacity, depending on whether it's selected or not.		Not tested
5	Remove focusable for icons in profile settings screen	Before, the screenreader would allow a user to focus the text and the icon of a SettingsRow element separately. On my personal phone, you can ONLY either select the text ("view privacy policy"), or the icon ("unpronounceable"). Now, I can't remove focusable as a property (seen in the Android emulator), because React Native Paper List.Icon element can't handle the focusable property, but I at least made the paernt element an accessibility element, so the icons should not be focusable, and only the entire SettingsRow is an accessibility element		Not tested
6	Read dropdown as "spinbutton" in profile settings screen	Before, the accessibilityRole read out loud by the screenreader for the Developer Zone button was "button". Now, I added the text of "spinbutton" to the end of the accessibility text, since the React Native Paper element List.Accordion can't handle accessibilityRole properties.		Not tested

[sebastianbarry]

@JGreenlee I added this to your review board, if either you or @jiji14 have time I'd really appreciate you taking a look and providing feedback! :)

Next step is to test on a physical device. My test phone does not have the software update to support the current staging release of OpenPATH, so I need to work with @jiji14 to use theirs. However, I am not sure what's the process of doing this without pushing it to staging. @Abby-Wheelis or @JGreenlee do you know the process for getting the app to run with our custom code and test travel data, so that Jiji and I can test what the screen reader actually reads?

[JGreenlee]

I only know how to do it for Android, but you can either:

1. install the devapp APK on a physical Android device and connect to the devapp server via local IP address (the phone must be on the same Wi-Fi as the machine running the devapp server)
2. build the app with cordova, locate the output apk and install it on the Android phone (either by using ADB or manually copying the apk)

[jiji14]

@sebastianbarry

I will test with an Android phone and comment here after I am done testing. I love how your summary is organized!

[JGreenlee]

Thanks for taking on this work and addressing a good number of these issues.
This review is not conclusive and we still have testing to do -- but I do want to leave an initial batch of comments here

[JGreenlee]

remove this file please

[sebastianbarry]

Deleted d5b7b51

[jiji14]

@sebastianbarry Here are the test results! I recorded them for your reference. Please let me know if you have any questions!

Test Device Information

Android - Samsung Galaxy S20

1. Logout button acts as 1 button

FAIL

Logout text is still separated

IMG_0.66.MOV

2. Unpronounceable metrics card icons on Dashboard

PASS

IMG_5.MOV

3. Remove non-character characters, like 👇

PASS

IMG_4.MOV

4. BottomNavigation buttons read double: "Profile Profile"

FAIL

Tab buttons still read double ex) Label tab Label Label

IMG_0.MOV

5. Remove focusable for icons in profile settings screen

PASS

IMG_6.MOV

6. Read dropdown as "spinbutton" in profile settings screen

PASS

It says Developer zone spinbutton button. I think it would be better if it just says Developer zone spinbutton but at least it says spinbutton!

IMG_0.31.MOV

[sebastianbarry]

@jiji14 Great testing!! I really appreciate your detail and videos, very helpful!

For the failed tests...:

Test 1: Logout button acts as 1 button

I fixed this in b94abff . It should now display as one single button, like the settings row buttons do. I modeled it after them, by changing the type of element that the logout button is.

Test 4: BottomNavigation buttons read double: "Profile Profile"

This issue may be out of our control. I haven't changed anything, although when I test with my iPhone emulator, it does not read "Label tab label label... etc." It only reads "Label tab, Dashbaord tab, profile tab". This makes me think that what the screen reader reads is dependent upon the device's built in screen reader, so it is different from iOS to Android. I think the solution will require creating a pull request to react-native-paper's codebase. @JGreenlee Is this something we want to pursue?

Screen.Recording.2024-01-22.at.10.32.46.AM.mov

[JGreenlee]

@sebastianbarry Can you toggle the target branch to master and back to service_rewrite_2023?
I want to verify that I resolved merge conflicts correctly

[sebastianbarry]

@JGreenlee Did it! let me know if it worked :)

[JGreenlee]

I'm not sure what's going on with this PR - all of my commits are showing up in the diff even though they exist in the target service_rewrite_2023. And the merge conflicts still show up.

Either way, @sebastianbarry could you actually change it back to master again ? (and leave it this time since master is now up-to-date!)

[sebastianbarry]

@JGreenlee I rebased it back to master! Looks like the only commit you made on this PR now is that one merge commit. There were 3 merge conflicts when I moved it over, but I took care of those since they were very simple.

[JGreenlee]

Glad that resolved itself! However, the 'build' test is now failing.
I haven't seen that happen before.

---

Edit: It seems like the qrcode scanner Cordova plugin might be the culprit?

[JGreenlee]

I just tried testing this branch in the devapp on an Android phone with a screenreader. TalkBack did not read anything at all and I couldn't navigate to anything by swiping left/right.

Confused, I tried TalkBack on the latest NREL OpenPATH. I got the same result.
I tried the current master on the devapp, and I also tried master at the point when the previous feature branch was merged in 342e190.

On all of these, TalkBack was not reading anything. Talkback works for other apps on the phone.

It didn't used to be like this, so either we had a major accessibility regression at some point (at least for Android), or I'm not remembering correctly how I did it before.

[JGreenlee]

When I get time, I will go back even further and see if I can either isolate the regression, or determine that there is no regression

[sebastianbarry]

At least for my iOS simulator, the accessibility inspector (simulating VoiceOver) is working on screen_reader branch

As for the build fail, I'm not sure what that is. I'm not even quite sure what the test fail error message is; is it this?

npm ERR! code 128
npm ERR! An unknown git error occurred
npm ERR! command git --no-replace-objects ls-remote ssh://git@github.com/e-mission/cordova-unified-logger.git
npm ERR! git@github.com: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.

[JGreenlee]

I haven't been able to test this out on a physical device, but Jiji and Sebastian both tested. The code changes look good to me so I will push it through to 'ready for review'

[shankari]

The current error is

Failed to restore plugin "cordova-plugin-em-unifiedlogger". You might need to try adding it again. Error: CordovaError: Failed to fetch plugin git+https://github.com/e-mission/cordova-unified-logger.git#v1.3.6 via registry.
Probably this is either a connection problem, or plugin spec is incorrect.
Check your connection and plugin name/version/URL.
CordovaError: Error: npm: Command failed with exit code 128 Error output:
npm ERR! code 128
npm ERR! An unknown git error occurred
npm ERR! command git --no-replace-objects ls-remote ssh://git@github.com/e-mission/cordova-unified-logger.git
npm ERR! git@github.com: Permission denied (publickey).
npm ERR! fatal: Could not read from remote repository.
npm ERR! 
npm ERR! Please make sure you have the correct access rights
npm ERR! and the repository exists.

Re-running to check if that resolves the error and passes the tests

[sebastianbarry]

@JGreenlee @shankari it passed! Thank you for your help

[shankari]

To summrarize the current state:

• it is not clear if this even works on android, however, this may not be a regression since it was not working even without these changes
• BottomNavigation buttons read double: "Profile Profile": is not fixed on android but is fixed on iOS. @sebastianbarry thinks that this will need changes to React Native Paper, but I don't see any investigation/evidence around this.
• we need feedback about emojis
• we need feedback about spinbutton

Regardless, it looks like it at least works on iOS, and is a step forward, so I will plan on reviewing + merging. @sebastianbarry can you update the issue with the current status so that we can finish polishing this later?

[shankari]

@sebastianbarry just a few comments to fix. The main change that I would really like to include is the i18n

[sebastianbarry]

Undoing desc and descriptionText changes in www/js/control/SettingRow.tsx

These are old changes from before I took over the PR, I can go ahead undo them

[sebastianbarry]

@shankari I addressed your comments. I think once we pass the tests, we should be good to merge!!