-
Notifications
You must be signed in to change notification settings - Fork 14
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
2173: Send language to iOS VoiceOver #2910
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR!
Can you explain a little bit more when we have to set the language? Whenever the language is different than the system language, right? Wouldn't that include stuff like the webview as well? How did you check where you need to set this?
Exactly, we have to set the language for components that should be read out in the app language as opposed to the system language. I checked it manually by going through the app on my phone with VoiceOver enabled. Webview seems to be doing this correctly already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice job! Not tested
Code Climate has analyzed commit ab734f1 and detected 1 issue on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 60.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 74.2% (0.0% change). View more on Code Climate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR, code looks good. Not tested, because i do not have a iphone.
💭 Should handle most cases properly, but we sometimes have edge cases, that still are handled wrongly, e.g. news that are send in german for all languages or things link this.
} | ||
} | ||
|
||
return { text: previousRoute.name, language: undefined } // system language |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔧 Feel free to fix this CodeScene issue, if you are motivated.
Short description
As part of the test of our accessibility from Pfennigparade a while ago, we got the feedback that the app doesn't send the language chosen in the app to OS so that you might get e.g. German texts read with an English pronunciation.
Android does a good job of that natively already, and the prop accessibilityLanguage can be used to achieve the same thing in iOS.
Proposed changes
Side effects
Testing
Alternatively, you can edit the
HeaderBox
to show the title and the language, and that should be good enough for a test.Resolved issues
Fixes: #2173