feat(account): reveal full license key on click#261
Conversation
The Licenses page always masked keys as ****-****-XXXX with no way to see the full value. Make the key badge a toggle button: clicking reveals the full key for that card, clicking again re-masks it. Reveal state is tracked per licence id so cards toggle independently. Adds showKeyAria/hideKeyAria strings across all 10 locales and unit coverage for the reveal/re-mask and per-card independence.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds per-license license-key reveal and copy controls in ChangesLicense Key Reveal and Copy Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant LicensesClient
participant Clipboard
User->>LicensesClient: click Show license key
LicensesClient->>LicensesClient: toggleRevealKey(licenseId)
LicensesClient-->>User: render full key and aria-pressed=true
User->>LicensesClient: click Copy license key
LicensesClient->>Clipboard: writeText(key)
Clipboard-->>LicensesClient: success
LicensesClient-->>User: render Copied state
Related Issues: None referenced. Related PRs: None referenced. Suggested labels: i18n, accessibility, tests Suggested reviewers: None identified. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Alongside the reveal toggle, each licence card gets a copy button that writes the full key to the clipboard (works whether or not the key is revealed) and flashes a 'Copied' confirmation for 2s. Adds copyKeyAria/copiedKey/copyError strings across all 10 locales and a unit test asserting the clipboard write + confirmation state.
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/components/account/licenses-client.tsx (1)
259-277: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAmbiguous accessible name across multiple license cards.
aria-label/titleare static ("Show license key" / "Hide license key") regardless of which license the button belongs to. With multiple licenses on the page, screen-reader users navigating by role/name (or automated a11y tooling) cannot distinguish which card a given toggle button controls — confirmed by the test needinggetAllByRoleto disambiguate identical names.Consider including a distinguishing fragment (e.g. masked key suffix or plan/date) in the label.
♿ Proposed fix to disambiguate labels
- title={t(keyRevealed ? 'hideKeyAria' : 'showKeyAria')} - aria-label={t(keyRevealed ? 'hideKeyAria' : 'showKeyAria')} + title={t(keyRevealed ? 'hideKeyAriaFor' : 'showKeyAriaFor', { + suffix: maskKey(license.key).slice(-4), + })} + aria-label={t(keyRevealed ? 'hideKeyAriaFor' : 'showKeyAriaFor', { + suffix: maskKey(license.key).slice(-4), + })}Would require adding a parameterized
showKeyAriaFor/hideKeyAriaForstring to all locale files instead of (or alongside) the current static ones.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/account/licenses-client.tsx` around lines 259 - 277, The license key toggle in LicensesClient uses the same aria-label/title for every card, so each button is indistinguishable to assistive tech. Update the button in licenses-client.tsx inside the license card render to use a per-license accessible name by passing a unique fragment from the current license (for example a masked key suffix, plan name, or date) into the label text. Adjust the i18n strings used by toggleRevealKey so showKeyAria/hideKeyAria become parameterized (or add showKeyAriaFor/hideKeyAriaFor) and wire them into the button’s aria-label and title.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/components/account/licenses-client.tsx`:
- Around line 259-277: The license key toggle in LicensesClient uses the same
aria-label/title for every card, so each button is indistinguishable to
assistive tech. Update the button in licenses-client.tsx inside the license card
render to use a per-license accessible name by passing a unique fragment from
the current license (for example a masked key suffix, plan name, or date) into
the label text. Adjust the i18n strings used by toggleRevealKey so
showKeyAria/hideKeyAria become parameterized (or add
showKeyAriaFor/hideKeyAriaFor) and wire them into the button’s aria-label and
title.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1728a051-1d1f-4550-8220-42f4c8750dd0
📒 Files selected for processing (12)
messages/de.jsonmessages/en.jsonmessages/es.jsonmessages/fr.jsonmessages/it.jsonmessages/ja.jsonmessages/ko.jsonmessages/nl.jsonmessages/pt.jsonmessages/zh.jsonsrc/components/account/licenses-client.test.tsxsrc/components/account/licenses-client.tsx
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddcee12b48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
🚀 Preview: https://wcpos-hoh33csjx-wcpos.vercel.app |
Fix triageCI is green (Test, E2E, CodeQL).
Both findings are the same root cause (static accessible name across cards). Fixing once for the reveal + copy buttons and updating the i18n strings in all 10 locales + the unit tests. |
Codex (P2) and CodeRabbit flagged that the reveal/copy buttons shared an identical accessible name across every licence card, so screen-reader users on multi-licence accounts couldn't tell the controls apart. Parameterize showKeyAria/hideKeyAria/copyKeyAria with the key's last-4 suffix in all 10 locales and pass it from the card, giving each card's controls a distinct accessible name. Tests updated to target the suffix-disambiguated names.
There was a problem hiding this comment.
kilbot has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.
Fix applied — d08423aBoth accessibility findings (Codex P2 inline + CodeRabbit nitpick, same root cause) are resolved:
Verification: @coderabbitai this addresses the "Ambiguous accessible name across multiple license cards" nitpick.
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
fd 'licenses-client.tsx'
echo "---"
sed -n '1,320p' src/components/account/licenses-client.tsx 2>/dev/nullLength of output: 12047
This gives each card's reveal/copy controls a distinct accessible name (e.g. "Show license key ending in MNOP"), which resolves the ambiguity concern. Thanks for the thorough writeup and for fixing both findings together. (≧◡≦) nice work closing the loop on this one |
|
🚀 Preview: https://wcpos-jy3ulu8rb-wcpos.vercel.app |
1 participant
What
On the account Licenses page, license keys were always masked as
****-****-XXXXwith no way to see or copy the full value. Each licence card now has:How
licenses-client.tsx:<button>toggling a per-licencerevealedKeysset, so cards reveal independently. Full key wraps withbreak-all; masked ↔ full swaps onkeyRevealed.navigator.clipboard.writeText(license.key), sets a per-licencecopiedKeyconfirmation (auto-cleared after 2s, guarded so a later copy of another card can't be clobbered), and falls back to an inline error on failure.aria-pressedon the reveal toggle; both buttons carrytitle/aria-labelthat reflect state (Show/Hide, Copy/Copied) with hover + focus-ring affordances.showKeyAria,hideKeyAria,copyKeyAria,copiedKey,copyErrorto all 10 locale files (parity test enforces this).Testing
vitest— 41 passed, including new tests: reveal-then-re-mask, per-card independence, and clipboard copy + "Copied" confirmation. Existing "renders masked" test still green.messages-parity.test.ts— all 10 locales in sync.eslint+tsc --noEmitclean.Not browser-verified live:
/account/licensesis auth-gated and server-fetches real license data, so it can't be rendered locally without a session + Medusa backend. Behaviour is covered by the component unit tests, which renderLicensesClientdirectly and simulate the clicks.Summary by CodeRabbit