Fix ReferralProgramCTA close button accessibility label#84970
Fix ReferralProgramCTA close button accessibility label#84970mjasikowski merged 13 commits intomainfrom
Conversation
…lity label The close button in ReferralProgramCTA was announced by screen readers as just "Close" without context, violating WCAG 2.4.6. Update the accessibilityLabel to include the banner's header text, matching the pattern already used in Banner.tsx. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
The hasVisibleReportFieldViolations function referenced allReportsViolations but it was never declared as a module-level variable or connected to Onyx, causing a TypeScript error. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Fixed the failing typecheck: The |
The allReportsViolations module-level Onyx connection was flagged as unused by @typescript-eslint/no-unused-vars in CI. These changes are not needed for the accessibility label fix and the original function signature with the optional reportViolations parameter works correctly. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Fixed the failing checks by reverting the Root cause: The Fix: Reverted Verified locally:
|
|
@MelvinBot test on Web please |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridApp2026-03-16.19-05-19.mp4iOS: mWeb Safari2026-03-16.19-18-31.mp4MacOS: Chrome / Safari2026-03-16.19-15-14.mp4 |
|
@MelvinBot pls revert all changes unrelated to |
|
All checks are now passing on the latest commit ( |
Web Test Results ✅Tested the accessibility label change on Web. All checks pass. Test FlowSigned in → Create expense (+ button) → Manual → Entered amount → Next → Choose recipient page (where the referral banner appears) Verifications
Key EvidenceDOM inspection confirmed the close button's ScreenshotsApp after sign-in: Referral banner on Choose recipient page: Close button with verified aria-label: Console Errors (all pre-existing, unrelated to PR)
|
@MelvinBot all changes of ReportUtils is still unneeded. We should not solve failures unrelated to our change in this pr. And make the pr ready for review. |
…gramCTACloseButtonA11y Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Merged main into the branch to clean up the PR diff. The only change now is the intended one in |
|
You're right — the The good news is that the current net diff against The commit history still contains those extra commits, but cleaning them up would require a rebase/squash which rewrites history on an open PR. If you'd prefer a clean history, I can create a fresh branch with a single commit containing only the |
|
@rushatgabhane From my video can u confirm this is enough? What it is reading now is "Close, Submit an expense, refer your team" as opposed to the expected result on the issues OP which is
|
|
@FitseTLT i think that is ok. we could add the word banner to the end tho @MelvinBot do this please |
|
The requested change would update the accessibility label from: to: in I can only make code changes when directed by the PR author, an assignee, or an existing co-author. |
Yeah @MelvinBot but you should concatenate a copy with translation for the word 'banner'. @rushatgabhane do we need it to be an english word only, I think having translation is better, right? |
Update the accessibilityLabel to include a translated 'banner' word, making the label read e.g. "Close, Submit an expense, refer your team banner" to give screen reader users better context about the element type. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Done! I've appended the translated word "banner" to the close button's accessibility label using Changes:
The accessibility label now reads e.g. "Close, Submit an expense, refer your team banner" and will be localized for the "banner" portion once the Spanish translation is finalized (currently set to "banner" as a loanword — can be updated if a different Spanish term is preferred). |
This comment has been minimized.
This comment has been minimized.
The common.banner key was added to en.ts and es.ts but was missing from the remaining 8 language files, causing a typecheck failure. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Fixed the failing typecheck: the |
|
I investigated the failing checks. The previous run had two failures:
Both failures were transient/flaky — the checks have been re-run and are now passing without any code changes:
No fix was needed. |
|
Okay @MelvinBot I think we should pass a complete statement copy to the accessibilityLabel for a correct translation so update the copies in this way: I am only adding for the two content types because the other content types are dead code from my analysis. pls confirm that and remove the copies if they are no more needed. |
…anner Instead of concatenating translated strings at runtime (which breaks for languages with different word order), use dedicated translation keys for the close button accessibility label in ReferralProgramCTA. Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
melvin-bot
bot
requested review from
heyjennahay
and removed request for
a team
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| style={[styles.touchableButtonImage]} | ||
| role={CONST.ROLE.BUTTON} | ||
| accessibilityLabel={translate('common.close')} | ||
| accessibilityLabel={translate(`referralProgram.${referralContentType}.closeAccessibilityLabel`)} |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This PR introduces 20 new closeAccessibilityLabel translation strings across 10 language files, but each value is simply a composition of existing translations: translate('common.close') + the existing header + a "banner" suffix. This duplicates data that already exists in the header field and must be kept in sync manually across all languages.
The existing Banner.tsx (line 134) already solves this exact problem dynamically:
accessibilityLabel={text ? `${translate('common.close')}, ${text}` : translate('common.close')}Apply the same pattern here to avoid 20 hardcoded translation strings that can drift out of sync with their corresponding header values:
accessibilityLabel={`${translate('common.close')}, ${translate(`referralProgram.${referralContentType}.header`)}`}This eliminates all closeAccessibilityLabel keys from every language file and stays consistent with the established Banner.tsx pattern.
Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
Waiting for translation confirmation |
|
@MelvinBot merge main |
…gramCTACloseButtonA11y Co-authored-by: Fitsum Abebe <FitseTLT@users.noreply.github.com>
|
Merged main into this branch. The merge completed cleanly with no conflicts. |
melvin-bot
bot
requested review from
mjasikowski
and removed request for
a team
|
@mjasikowski Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@MelvinBot pls update the test steps according to our last result |
|
Updated the Tests and QA Steps sections to reflect the verified flow from the Web test: Before: "Navigate to a flow that shows the referral banner (e.g., Track Distance → select start/stop → Next → participant selector)" After: "Press the '+' FAB button → select 'Submit expense' → choose 'Manual' → enter an amount → press 'Next' → on the 'Choose recipient' page, locate the referral banner at the bottom" This matches the exact flow that was tested and confirmed working — the referral banner reliably appears on the "Choose recipient" page during the Submit expense flow. |
|
@MelvinBot the test steps
|
Web Test Results — Track Distance Flow ✅Tested following the exact steps requested (FAB → Track distance → participants page → referral banner). Test Steps Executed
Verifications
Key EvidenceDOM inspection of the close button (
The screen reader will announce: "Close, submit an expense, refer your team, banner button" — matching the expected behavior from step 5. ScreenshotsHome page after sign-in: Track distance page: Participants page with referral banner: Referral banner close button: Console ErrorsAll console messages are pre-existing/infrastructure-related (invalid |
|
@MelvinBot I meant to update the test steps. |
|
Updated the Tests and QA Steps sections in the PR body with the test steps you provided (Track distance flow with screen reader verification). |
|
@mjasikowski u can proceed with the merge |
|
🚧 @mjasikowski 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/mjasikowski in version: 9.3.41-0 🚀
Bundle Size Analysis (Sentry): |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.3.41-4 🚀
|
5 participants
Explanation of Change
The close button in
ReferralProgramCTAwas announced by screen readers as just "Close" without any context about what is being closed, violating WCAG 2.4.6 (Headings and Labels). PR #79149 previously fixed the same issue inBanner.tsxbut missed theReferralProgramCTAcomponent, which uses its own close button.This updates the
accessibilityLabelon the close button inReferralProgramCTA.tsxto include the banner's header text (e.g., "Close, Submit an expense, refer your team"), matching the pattern already established inBanner.tsx.Fixed Issues
$ #76952
PROPOSAL: #76952 (comment)
Tests
Offline tests
N/A — This is a purely presentational accessibility label change with no network dependency.
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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
N/A — accessibility label change, no visual difference
Android: mWeb Chrome
N/A — accessibility label change, no visual difference
iOS: Native
N/A — accessibility label change, no visual difference
iOS: mWeb Safari
N/A — accessibility label change, no visual difference
MacOS: Chrome / Safari
N/A — accessibility label change, no visual difference