fix: the input field is announced without entered value#82651
fix: the input field is announced without entered value#82651dangrous merged 12 commits intoExpensify:mainfrom
Conversation
|
Hey! I see that you made changes to our Form component. Make sure to update the docs in FORMS.md accordingly. Cheers! |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
Hmm the behaviour of this seems somewhat inconsistent, for example on iOS the label is announced twice where as on Android it's not announced atall while editing: Android_Native.moviOS_Native.mov |
I can't hear anything from your video. Could you please re-upload it? |
You can see what's being announced from the subtitles at the bottom of the screen. |
|
@Ollyws My Android simulator doesn't have the TallBack feature @heyjennahay could you please trigger to build adhoc then i can test with the real device? |
|
🚧 @dangrous has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
|
@Ollyws all yours |
|
I'll try to test on the real Android device today |
android-resize.mp4@Ollyws it's working fine with the real Android device |
|
@daleh I'm getting exactly the same behaviour on a real device as my earlier simulator video: Screen_Recording_20260306_094245_Expensify.Adhoc.mp4 |
|
@daledah Correct that the input value is anounced, but the problem is that the 'Phone or email' label is never anounced on Android but it is announced on iOS.
Not quite yet. |
@Ollyws So what is the expected behavior here? Should the ‘Phone or email’ label be announced or not? |
I think the announcement should include the label, role and value. Consistently for both Android and iOS. |
|
@Ollyws The root cause is that Android TalkBack (16.x) prioritizes reading the actual text content of |
|
The failing test is not related to our changes, i'll merge main to fix it soon |
src/components/TextInput/BaseTextInput/implementation/index.tsx
Outdated
Show resolved
Hide resolved
|
@Ollyws all yours |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp01_Android_Native.mp4Android: mWeb Chrome02_Android_Chrome.mp4iOS: HybridApp03_iOS_Native.mp4iOS: mWeb Safari04_iOS_Safari.mp4MacOS: Chrome / Safari |
Ollyws
left a comment
There was a problem hiding this comment.
Aside from #82651 (comment) LGTM. Approving to keep it moving.
dangrous
left a comment
There was a problem hiding this comment.
looks good to me except a) conflicts and b) let's stick to the new flow for the error stuff re: https://github.com/Expensify/App/pull/82964/changes - thanks!
src/components/TextInput/BaseTextInput/implementation/index.tsx
Outdated
Show resolved
Hide resolved
|
@dangrous i resolved conflict and updated, please check again |
|
🚧 @dangrous has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/dangrous in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|



Explanation of Change
Fixed Issues
$ #77547
PROPOSAL: #77547 (comment)
Tests
Prerequisites: the user is not signed in
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari