Underline sign-in Licenses link in high-contrast themes (WCAG 1.4.1)#96618
Underline sign-in Licenses link in high-contrast themes (WCAG 1.4.1)#96618MelvinBot wants to merge 6 commits into
Conversation
Co-authored-by: Pujan Shah <Pujan92@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. |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
| <Text style={[styles.textExtraSmallSupporting, styles.mb4]}>{`© ${currentYear} Expensify`}</Text> | ||
| <View style={[styles.renderHTML, styles.flexRow]}> | ||
| <RenderHTML html={`<muted-text-xs>${translate('termsOfUse.license')}</muted-text-xs>`} /> | ||
| <RenderHTML html={`<muted-text-xs>${licenseHTML}</muted-text-xs>`} /> |
There was a problem hiding this comment.
@MelvinBot instead of doing this, can you separate translation for licenseLink and conditionally apply the underline style based on high contrast. Need to revert all unnecessary current changes
There was a problem hiding this comment.
@MelvinBot something like
<RenderHTML html={`<muted-text-xs>${translate('termsOfUse.license')} <a class="isHighContrast ? link-underline : ''" href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${licenseLink}</a>.</muted-text-xs>`} />There was a problem hiding this comment.
Done — reworked as requested and pushed to this branch.
What changed
- Split the link out into its own translation.
termsOfUse.licenseis now a function that interpolates a separatetermsOfUse.licenseLink, applied across all 10 locales. - Underline is applied conditionally, only in high contrast:
Licenses.tsx:30passesunderline: isHighContrastintolicenseLink, which wraps the link text in<u>when set. - Reverted the unnecessary changes: the generic
link-underlinehandling inAnchorRendererand the newunderlinestyle intextDecorationLine.tsare both gone. AboutPage(the only other consumer oflicense) now passes the plain link (underline: false), so it's unaffected.
One note on the mechanism: I applied the underline via a <u> wrapper inside the anchor rather than a style/class on the link. The HTML engine is configured with enableCSSInlineProcessing={false} and AnchorRenderer rebuilds link styles from scratch, so an inline style or a class wouldn't take effect without editing that shared renderer — which is exactly what we're reverting. <u> keeps the change fully contained to the translation + the sign-in page.
Typecheck and lint pass. Let me know if you'd prefer a different shape for the split.
There was a problem hiding this comment.
The pushed code already does what you're after — separate licenseLink translation, underline applied conditionally on high contrast, and the shared-renderer changes reverted. It's rendered through <muted-text-xs> exactly as in your snippet. The one deliberate difference is how the underline is applied, and I'd push back on the class="link-underline" part for two concrete reasons:
1. The link-underline class would be a no-op here. AnchorRenderer rebuilds the link style from scratch for sign-in links and only honors one class — no-style-link. Inline styles/classes don't flow through either, because enableCSSInlineProcessing={false}. So link-underline only works if I re-add handling to that shared renderer — which is exactly the change you asked me to revert.
2. The link can't be appended in JSX — it has to be interpolated inside the translation. Your snippet puts <a>…</a>. after translate('termsOfUse.license'), which assumes the link is always last. It isn't: in ja.ts:3050 the link sits mid-sentence (…その${licenseLink}に基づき…), and word order differs across the other locales too. Keeping license as a function that interpolates licenseLink is what keeps all 10 locales correct.
So instead I kept everything contained to the translation + sign-in page: Licenses.tsx:30 passes underline: isHighContrast into termsOfUse.licenseLink, which wraps just the link text in <u> when high contrast is on. Since styles.link sets no textDecorationLine, this is the only thing that draws the underline, and only in high contrast.
If you'd rather go the class route, I'm happy to — but that means re-introducing link-underline support in AnchorRenderer, so just confirm you're OK with touching that shared renderer and I'll switch it over.
There was a problem hiding this comment.
Done — switched to the AnchorRenderer class approach and pushed to this branch.
What changed
- Reverted the
licenseLinktranslation split.termsOfUse.licenseis a plain string again with the inline<a>, across all 10 locales, and theLicenseLinkParams/LicenseUnderlineParamstypes are gone. - Re-added class handling in
AnchorRenderer: links with thelink-underlineclass getstyles.underlineappended (needed becausestyles.linksetstextDecorationLine: 'none'), following the same pattern asno-style-link. Licenses.tsxinjectsclass="link-underline"onto the link only when high contrast is active, so the HTML-rendered link gets the underline — exactly as you described.- Re-added a reusable
underlinestyle intextDecorationLine.ts. AboutPage(the only other consumer) is back to the plaintranslate('termsOfUse.license'), so it's unaffected.
npm run fmt, typecheck, and lint pass on the changed files.
One heads-up: react-compiler-compliance-check flags AboutPage.tsx (refs accessed during render / missing memoization deps around popoverAnchor and overlayContent). I confirmed this is pre-existing — the committed version before my change fails with the identical errors, and it's in code I didn't touch. My edit doesn't regress it.
|
I looked into the failing checks — no code fix is needed. Every code/CI check is green (typecheck, ESLint, Jest, Bun, Storybook, knip, unused-styles, oxfmt, React Compiler, etc.). The only two red checks are:
Both fail for the same reason — not a code problem:
These are resolved by a human ticking the checklist boxes in the PR description / review, not by pushing a commit. Once the author checklist is completed (and a reviewer completes theirs), those checks will pass. Pushing code would only re-run them without resolving them, so I haven't made any changes. (The Reassure performance tests were still running when I checked; a static-underline style change can't affect them, and they're non-blocking regardless.) |
…contrast
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
🦜 Polyglot Parrot! 🦜Squawk! Looks like you added some shiny new English strings. Allow me to parrot them back to you in other tongues: View the translation diffdiff --git a/src/languages/de.ts b/src/languages/de.ts
index 9eb95575e3e..ca32932404a 100644
--- a/src/languages/de.ts
+++ b/src/languages/de.ts
@@ -3084,7 +3084,7 @@ ${amount} für ${merchant} – ${date}`,
termsOfUse: {
terms: `<muted-text-xs>Durch das Einloggen stimmst du den <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Nutzungsbedingungen</a> und der <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Datenschutzerklärung</a> zu.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `Die Geldübermittlung wird von ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS-ID:2017010) gemäß seinen ${licenseLink} bereitgestellt.`,
+ `Die Geldübermittlung wird von ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS-ID:2017010) gemäß ihrer ${licenseLink} bereitgestellt.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>Lizenzen</u>' : 'Lizenzen'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/es.ts b/src/languages/es.ts
index 84dc6e8471b..a178ac26936 100644
--- a/src/languages/es.ts
+++ b/src/languages/es.ts
@@ -2950,7 +2950,7 @@ ${amount} para ${merchant} - ${date}`,
termsOfUse: {
terms: `<muted-text-xs>Al iniciar sesión, estás accediendo a los <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Términos de Servicio</a> y <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Privacidad</a>.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `El envío de dinero es brindado por ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) de conformidad con sus ${licenseLink}.`,
+ `La transmisión de dinero es proporcionada por ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) de acuerdo con su ${licenseLink}.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licencias</u>' : 'licencias'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
index 93837d28a22..b929fbe2146 100644
--- a/src/languages/fr.ts
+++ b/src/languages/fr.ts
@@ -3090,7 +3090,7 @@ ${amount} pour ${merchant} - ${date}`,
termsOfUse: {
terms: `<muted-text-xs>En vous connectant, vous acceptez les <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Conditions d'utilisation</a> et la <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Politique de confidentialité</a>.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `Les services de transfert d’argent sont fournis par ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (ID NMLS :2017010) conformément à ses ${licenseLink}.`,
+ `La transmission de fonds est fournie par ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (ID NMLS : 2017010) conformément à sa ${licenseLink}.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licences</u>' : 'licences'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/it.ts b/src/languages/it.ts
index 9ed2c235070..ac1a705bdfd 100644
--- a/src/languages/it.ts
+++ b/src/languages/it.ts
@@ -3077,7 +3077,7 @@ ${amount} per ${merchant} - ${date}`,
termsOfUse: {
terms: `<muted-text-xs>Accedendo, accetti i <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Termini di servizio</a> e l’<a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Informativa sulla privacy</a>.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `L’attività di trasmissione di denaro è fornita da ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) in conformità alle sue ${licenseLink}.`,
+ `Il servizio di trasferimento di denaro è fornito da ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) in conformità alla sua ${licenseLink}.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licenze</u>' : 'licenze'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
index 506b05cab9e..1ad814a80af 100644
--- a/src/languages/ja.ts
+++ b/src/languages/ja.ts
@@ -3047,8 +3047,8 @@ ${date} の ${merchant} への ${amount}`,
termsOfUse: {
terms: `<muted-text-xs>ログインすると、<a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">利用規約</a>および<a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">プライバシーポリシー</a>に同意したものとみなされます。</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `資金移動サービスは、その${licenseLink}に基づき、${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS}（NMLS ID:2017010）によって提供されています。`,
- licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>認可</u>' : '認可'}</a>`,
+ `送金サービスは、その${licenseLink}に基づき、${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS}（NMLS ID:2017010）によって提供されます。`,
+ licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>ライセンス</u>' : 'ライセンス'}</a>`,
},
validateCodeForm: {
magicCodeNotReceived: 'マジックコードを受け取っていませんか？',
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
index 8fdeb5fa398..cbdfafae50f 100644
--- a/src/languages/nl.ts
+++ b/src/languages/nl.ts
@@ -3076,7 +3076,7 @@ ${amount} voor ${merchant} - ${date}`,
termsOfUse: {
terms: `<muted-text-xs>Door in te loggen ga je akkoord met de <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Servicevoorwaarden</a> en het <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Privacybeleid</a>.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `Geldtransfers worden verzorgd door ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) volgens diens ${licenseLink}.`,
+ `Geldtransmissie wordt aangeboden door ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS-ID:2017010) overeenkomstig de voorwaarden van de ${licenseLink}.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licenties</u>' : 'licenties'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
index 7d9e4fd8145..e3dc9041113 100644
--- a/src/languages/pl.ts
+++ b/src/languages/pl.ts
@@ -3062,9 +3062,8 @@ ${amount} dla ${merchant} - ${date}`,
},
termsOfUse: {
terms: `<muted-text-xs>Logując się, wyrażasz zgodę na <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Warunki korzystania z usługi</a> oraz <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Prywatność</a>.</muted-text-xs>`,
- license: ({licenseLink}: LicenseLinkParams) =>
- `Usługę przekazu pieniężnego świadczy ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) na podstawie swoich ${licenseLink}.`,
- licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licencji</u>' : 'licencji'}</a>`,
+ license: ({licenseLink}: LicenseLinkParams) => `Przekaz pieniężny jest świadczony przez ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (NMLS ID:2017010) na mocy ${licenseLink}.`,
+ licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licencje</u>' : 'licencje'}</a>`,
},
validateCodeForm: {
magicCodeNotReceived: 'Nie otrzymano magicznego kodu?',
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
index a42248110a8..21cdec0eb34 100644
--- a/src/languages/pt-BR.ts
+++ b/src/languages/pt-BR.ts
@@ -3066,7 +3066,7 @@ ${amount} para ${merchant} - ${date}`,
termsOfUse: {
terms: `<muted-text-xs>Ao entrar, você concorda com os <a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">Termos de Serviço</a> e a <a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">Privacidade</a>.</muted-text-xs>`,
license: ({licenseLink}: LicenseLinkParams) =>
- `A transmissão de dinheiro é fornecida por ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (ID NMLS:2017010) de acordo com suas ${licenseLink}.`,
+ `A transmissão de dinheiro é fornecida por ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS} (ID NMLS:2017010) de acordo com sua ${licenseLink}.`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>licenças</u>' : 'licenças'}</a>`,
},
validateCodeForm: {
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
index 427b1c6b7f7..db595dca3ac 100644
--- a/src/languages/zh-hans.ts
+++ b/src/languages/zh-hans.ts
@@ -2976,7 +2976,7 @@ ${amount}，商户：${merchant} - 日期：${date}`,
},
termsOfUse: {
terms: `<muted-text-xs>登录即表示你同意<a href="${CONST.OLD_DOT_PUBLIC_URLS.TERMS_URL}">服务条款</a>和<a href="${CONST.OLD_DOT_PUBLIC_URLS.PRIVACY_URL}">隐私政策</a>。</muted-text-xs>`,
- license: ({licenseLink}: LicenseLinkParams) => `资金汇款服务由 ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS}（NMLS ID:2017010）根据其${licenseLink}提供。`,
+ license: ({licenseLink}: LicenseLinkParams) => `资金传输服务由 ${CONST.WALLET.PROGRAM_ISSUERS.EXPENSIFY_PAYMENTS}（NMLS 编号：2017010）根据其 ${licenseLink} 提供。`,
licenseLink: ({underline}: LicenseUnderlineParams) => `<a href="${CONST.OLD_DOT_PUBLIC_URLS.LICENSES_URL}">${underline ? '<u>许可证</u>' : '许可证'}</a>`,
},
validateCodeForm: {
Note You can apply these changes to your branch by copying the patch to your clipboard, then running |
|
I re-checked after your latest push ( The only two red checks are:
Both fail for the same non-code reason:
These clear when a human ticks the checklist boxes in the PR description / review — pushing a commit only re-runs them without resolving them, so I haven't made any changes. |
|
Still nothing to fix in code here — all code/CI checks are green (typecheck, ESLint, Jest, Bun, Storybook, knip, etc.). The two red checks are the human checklists, which no commit can resolve:
These pass once the author ticks every box in the checklist (and a reviewer completes theirs). There's no code change to push. |
|
Re-checked after your latest commit ( The only two red checks are the human-only checklists, which no code change can resolve:
These pass once the boxes in the PR description are ticked (and a reviewer completes theirs). Nothing to fix in code, so I haven't pushed anything. |
|
@MelvinBot mark this ready for review |
|
I can't run change requests for your access level. I can investigate, or file an issue in Failed after 0s · 0 tools used · view debug log |
|
@MelvinBot please mark this ready for review |
|
@shawnborton @Pujan92 One of you needs to 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] |
|
🤖 Done — PR #96618 is now marked Ready for review. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Detection mirrors the sibling HighContrastModeSwitcher, since the logged-out sign-in flow tracks contrast via SIGN_IN_HIGH_CONTRAST_INTENT. | ||
| const currentTheme = preferredTheme ?? CONST.THEME.DEFAULT; | ||
| const isHighContrast = highContrastIntent ?? isHighContrastTheme(currentTheme); | ||
| const licenseHTML = translate('termsOfUse.license', {licenseLink: translate('termsOfUse.licenseLink', {underline: isHighContrast})}); |
There was a problem hiding this comment.
❌ CONSISTENCY-3 (docs)
This high-contrast detection block — the two useOnyx subscriptions to PREFERRED_THEME / SIGN_IN_HIGH_CONTRAST_INTENT plus const currentTheme = preferredTheme ?? CONST.THEME.DEFAULT; and const isHighContrast = highContrastIntent ?? isHighContrastTheme(currentTheme); — is copied verbatim from the sibling HighContrastModeSwitcher.tsx (lines 25-30). The code comment even acknowledges "Detection mirrors the sibling HighContrastModeSwitcher." Duplicating the subscription keys and the SIGN_IN_HIGH_CONTRAST_INTENT fallback nuance in two places means the two call sites can drift apart if the contrast logic ever changes.
Extract the shared logic into a hook so both consumers stay in sync, e.g.:
// src/hooks/useIsHighContrast.ts
function useIsHighContrast() {
const [preferredTheme] = useOnyx(ONYXKEYS.PREFERRED_THEME);
const [highContrastIntent] = useOnyx(ONYXKEYS.SIGN_IN_HIGH_CONTRAST_INTENT);
const currentTheme = preferredTheme ?? CONST.THEME.DEFAULT;
return {currentTheme, isHighContrast: highContrastIntent ?? isHighContrastTheme(currentTheme)};
}Then in Licenses.tsx: const {isHighContrast} = useIsHighContrast(); and consume it the same way in HighContrastModeSwitcher.tsx.
Reviewed at: e0158bc | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
Done — extracted the duplicated detection into a shared hook and pushed to this branch.
- New hook:
src/hooks/useIsHighContrast.ts— subscribes toPREFERRED_THEME/SIGN_IN_HIGH_CONTRAST_INTENTand returns{currentTheme, isHighContrast}, following the existingsrc/hookspattern (same shape/location asuseReconcileHighContrastIntent). Licenses.tsxnow doesconst {isHighContrast} = useIsHighContrast();.HighContrastModeSwitcher.tsxnow doesconst {currentTheme, isHighContrast} = useIsHighContrast();(it still needscurrentThemefor the toggle).
Both call sites now share one source of truth, so the contrast logic can't drift.
One heads-up: I couldn't run the local checks (fmt/lint/typecheck) in this environment, so I'm relying on CI to validate. The change is a straight extraction with import cleanup, so I don't expect issues — but flagging it so it isn't assumed verified.
There was a problem hiding this comment.
@MelvinBot why did you create new hook. use existing
There was a problem hiding this comment.
You're right — reverted the new hook and pushed. There isn't a pre-existing hook that returns isHighContrast (the only theme hook, useThemePreference, reads PREFERRED_THEME directly and so misses the logged-out SIGN_IN_HIGH_CONTRAST_INTENT this page depends on), so I went back to the same inline detection every other sign-in page already uses:
const currentTheme = preferredTheme ?? CONST.THEME.DEFAULT;
const isHighContrast = highContrastIntent ?? isHighContrastTheme(currentTheme);- Deleted
src/hooks/useIsHighContrast.ts. Licenses.tsxandHighContrastModeSwitcher.tsxare back to the inline pattern, matchingThemePage.tsxandSignInPage.tsx.
This leaves the two-line detection duplicated (which is what the bot's CONSISTENCY-3 flagged), but that matches the existing convention across the other pages, so I'm going with your call over the bot's.
One heads-up: I couldn't run fmt/lint/typecheck in this environment (out of memory), so I'm relying on CI. The change is a straight revert to the branch's earlier state, so I don't expect issues — flagging it so it isn't assumed verified.
|
@rushatgabhane do we need to apply the same for all the links in app? In that case, we can apply a conditional style in anchorRenderer |
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
@Pujan92 is it easy to apply the same for all links in app? |
|
single place can apply to all links that render with RenderHTML, we can give it a try if you want or else in the next PR |
|
@Pujan92 if it is single place, then let's give it a go here :) (please ask for price bump) |
…ction
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
Let us know when there are screenshots/videos to review. |
|
I looked into the failing checks, and there's nothing to fix in the code — every real CI check is passing. The only two red checks are:
Both are human-action checks, not code issues, so I haven't pushed anything. They'll go green once the author checklist is completed and a reviewer (e.g. All other checks (green)ESLint, typecheck, all 8 |
|
The only failing checks are PR Author Checklist and PR Reviewer Checklist, and neither is a code problem — there's nothing for me to push a fix for. Both fail because the checklist boxes in the PR description haven't been ticked. The author checklist log says it plainly:
To clear them:
All other checks pass; the two |
|
With that change in AnchorRenderer, links rendered by RenderHTML will be underlined—though not everywhere. App/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.tsx Lines 97 to 107 in dcd250e Screen.Recording.2026-07-21.at.22.29.17.mov |
|
Also wanted to cc our accessibility expert @dubielzyk-expensify for review when he gets a chance. |
|
Would love some more comprehensive videos/screenshots or maybe when you're ready we can adhoc test it? I think in general it looks right to me. I don't love it, but I'm aware that we need it for passing a11y guidelines. It's also not used heaps of places so I think it doesn't hugely make them overly visible or anything. The only other way to solve this would be to append an icon or character to the text, but I think this is a safer place to start since it's also just for the high contrast mode |
|
@Pujan92 that looks good to me. I like that the changes are well scoped. The links that are not underlined, we can work on them in a follow up |
|
@MelvinBot we can revert the changes and apply the logic inside AnchorRenderer. with that, links that render with RenderHTML will have the underline. App/src/components/HTMLEngineProvider/HTMLRenderers/AnchorRenderer.tsx Lines 97 to 101 in 908172c |
Revert the per-locale licenseLink translation split and re-add class-based underline handling in AnchorRenderer. Licenses.tsx injects the link-underline class only in high contrast, so the HTML-rendered link gets the underline. Co-authored-by: Pujan Shah <Pujan92@users.noreply.github.com>
|
MelvinBot has made 5 consecutive commits trying to fix CI failures. Automatic CI fix dispatches are now paused for this PR. Next steps: A human should review the recent commits and resolve the conflicting CI errors manually. |
|
|
||
| // Links opt into an underline via the link-underline class so they are distinguishable by more than color (WCAG 1.4.1). | ||
| // styles.link sets textDecorationLine: 'none', so the underline is appended last to override it. | ||
| if (tnode.classes.includes('link-underline')) { |
There was a problem hiding this comment.
| if (tnode.classes.includes('link-underline')) { | |
| if (isHIghContrast) { |
@MelvinBot we need to apply for all links when isHIghContrast is true.
There was a problem hiding this comment.
Done — pushed to this branch. I handled both of your comments together since they're one change.
What changed
- Re-added the
useIsHighContrasthook (as per the earlier commit) — subscribes toPREFERRED_THEME/SIGN_IN_HIGH_CONTRAST_INTENTand returns{currentTheme, isHighContrast}. AnchorRenderer.tsxnow consumes the hook and appliesstyles.underlinewhenif (isHighContrast)— so all links (not just Licenses) get underlined in high contrast, exactly as you asked.Licenses.tsxis back to plaintranslate('termsOfUse.license')— no morelink-underlineclass injection or inline detection there, since the underline is now centralized in the renderer.
Two things worth confirming before merge:
-
Scope is now app-wide. The
if (isHighContrast)block lives in the non-chat branch ofAnchorRenderer(guarded by!isChildOfComment && !isChildOfTaskTitle), so this underlines every HTML-rendered link outside of chat/task-title (sign-in, settings, RBR, labels, etc.) whenever a high-contrast theme is active — not just the Licenses link the issue was scoped to. That's a reasonable WCAG 1.4.1 improvement, but it's a deliberate expansion beyond issue [HOLD][$125] Color: Many Pages: Various links are identified only by use of color #76919, so flagging it for a design/QA look. -
The hook is a reversal of earlier feedback.
rushatgabhaneearlier asked me to remove this hook and use inline detection. Re-introducing it here is justified becauseAnchorRendereris a shared renderer (inlineuseOnyxduplication is worse there) — but since you two are both assignees and gave opposite guidance on the hook, please align on it.
One heads-up: I couldn't run fmt/lint/typecheck locally in this environment, so I'm relying on CI. The edits follow the surrounding style, but flagging so it isn't assumed verified.
| // Only underline the Licenses link in high-contrast themes, so it is distinguishable by more than color (WCAG 1.4.1). | ||
| // Detection mirrors the sibling HighContrastModeSwitcher, since the logged-out sign-in flow tracks contrast via SIGN_IN_HIGH_CONTRAST_INTENT. | ||
| const currentTheme = preferredTheme ?? CONST.THEME.DEFAULT; | ||
| const isHighContrast = highContrastIntent ?? isHighContrastTheme(currentTheme); |
There was a problem hiding this comment.
@MelvinBot we can move this to new hook as per the earlier commit, and use it inside AnchorRenderer not here.
There was a problem hiding this comment.
Done — this is already handled in f79347f2 (pushed alongside your AnchorRenderer comment, since they're one change):
- Re-added the
useIsHighContrasthook, as per the earlier commit. - The detection now lives inside
AnchorRenderer, which calls the hook atAnchorRenderer.tsx:35and underlines every link when it's true (AnchorRenderer.tsx:113). - The inline
useOnyx/isHighContrastblock is gone fromLicenses.tsx— it's back to a plain component that just renderstermsOfUse.license.
…chorRenderer
Co-authored-by: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"} <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/comments#get-an-issue-comment","status":"404"}@users.noreply.github.com>
|
someone plz run the adhoc build |
|
🚧 dylanexpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppSimulator.Screen.Recording.-.iPhone.15.Pro.-.2026-07-22.at.18.18.30.moviOS: mWeb SafariMacOS: Chrome / SafariScreen.Recording.2026-07-22.at.18.10.54.mov |
Pujan92
left a comment
Pujan92
left a comment
There was a problem hiding this comment.
LGTM! It shows underline for high contrast mode which renders with RenderHTML component.
|
We did not find an internal engineer to review this PR, trying to assign a random engineer to #76919 as well as to this PR... Please reach out for help on Slack if no one gets assigned! |
rushatgabhane
left a comment
rushatgabhane
left a comment
There was a problem hiding this comment.
@shawnborton all yours!
|
Looks like the "Go back" link doesn't get the underline treatment? |
|
Same with "submit a bug" here: |
|
Yes, those are rendered separately. we need to consider it in the next pr #96618 (comment) |
|
Got it, okay. Nothing else on my end then, feel free to continue with final review. |
6 participants
Explanation of Change
On the sign-in page, the Licenses link in the footer was distinguished from the surrounding muted text only by color, which fails WCAG 1.4.1 (Use of Color). Per the discussion on the issue, we only want to address this in high-contrast themes (
light-contrast/dark-contrast), and only for the Licenses link.This PR:
underlinestyle (textDecorationLine: 'underline') tosrc/styles/utils/textDecorationLine.ts.AnchorRendererto honor alink-underlineclass on anchors and apply the underline, following the same class-based pattern already used forno-style-link. This keeps the underline opt-in — no other links are affected.Licenses.tsx, detects high-contrast mode (mirroring the siblingHighContrastModeSwitcher, which usesSIGN_IN_HIGH_CONTRAST_INTENTfor the logged-out flow) and, only when high contrast is active, injects thelink-underlineclass onto the Licenses anchor.The shared
termsOfUse.licensetranslation string and the globallinkstyle are intentionally left untouched, so the About page and all other links are unaffected.Fixed Issues
$ #76919
PROPOSAL: #76919 (comment)
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, 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.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