fix: localize external link aria labels#18
Conversation
Deploying afx-team with
|
| Latest commit: |
0760e75
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0cf54061.afx-team.pages.dev |
| Branch Preview URL: | https://codex-aria-label-i18n.afx-team.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesAria-label localization
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant I18nManager
participant DOMElement
User->>I18nManager: switch language (zh/en)
I18nManager->>DOMElement: read data-aria-zh / data-aria-en
I18nManager->>DOMElement: setAttribute('aria-label', localizedValue)
I18nManager->>DOMElement: update textContent via data-zh/data-en
Related issues: Suggested labels: accessibility, i18n, tests Suggested reviewers: afx-team maintainers familiar with 🐇 A rabbit hops through links so bright, 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
afc163
force-pushed
the
codex/aria-label-i18n
branch
from
4e5bc39 to
947f675
Compare
♻️ Preview destroyed
🤖 Powered by surge-preview |
|||||||||
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces localized ARIA labels for external links to improve accessibility. It adds data-aria-zh and data-aria-en attributes to anchor tags in index.html, updates js/i18n.js to dynamically toggle the aria-label based on the active language, and adds a test suite to verify this behavior. Review feedback points out a minor inconsistency in index.html where the square brackets are missing from the Chinese ARIA attributes for the 'Mako Next' link.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| <div> | ||
| <h3 class="font-semibold text-base mb-1 tracking-wide"> | ||
| <a href="https://mp.weixin.qq.com/s/CdCvCzfycme6rw4OKa_1aA" target="_blank" rel="noopener noreferrer" class="text-gray-700 hover:text-black" aria-label="MCP 在蚂蚁前端的落地之旅 - 在新窗口打开" data-zh="MCP 在蚂蚁前端的落地之旅" data-en="MCP Landing Journey in Ant Frontend">MCP 在蚂蚁前端的落地之旅</a> | ||
| <a href="https://mp.weixin.qq.com/s/CdCvCzfycme6rw4OKa_1aA" target="_blank" rel="noopener noreferrer" class="text-gray-700 hover:text-black" aria-label="MCP 在蚂蚁前端的落地之旅 - 在新窗口打开" data-aria-zh="MCP 在蚂蚁前端的落地之旅 - 在新窗口打开" data-aria-en="MCP Landing Journey in Ant Frontend - Open in new window" data-zh="MCP 在蚂蚁前端的落地之旅" data-en="MCP Landing Journey in Ant Frontend">MCP 在蚂蚁前端的落地之旅</a> |
There was a problem hiding this comment.
The aria-label and data-aria-zh attributes are missing the square brackets [ and ] around Mako Next, which is inconsistent with the visible text [Mako Next]: ... and the English equivalent data-aria-en="[Mako Next]: ...". Adding them ensures consistency across all localized labels.
| <a href="https://mp.weixin.qq.com/s/CdCvCzfycme6rw4OKa_1aA" target="_blank" rel="noopener noreferrer" class="text-gray-700 hover:text-black" aria-label="MCP 在蚂蚁前端的落地之旅 - 在新窗口打开" data-aria-zh="MCP 在蚂蚁前端的落地之旅 - 在新窗口打开" data-aria-en="MCP Landing Journey in Ant Frontend - Open in new window" data-zh="MCP 在蚂蚁前端的落地之旅" data-en="MCP Landing Journey in Ant Frontend">MCP 在蚂蚁前端的落地之旅</a> | |
| <a href="https://github.com/umijs/mako/issues/1872" target="_blank" rel="noopener noreferrer" class="text-gray-700 hover:text-black" aria-label="[Mako Next]: A new bundler on top of Turbopack - 在新窗口打开" data-aria-zh="[Mako Next]: A new bundler on top of Turbopack - 在新窗口打开" data-aria-en="[Mako Next]: A new bundler on top of Turbopack - Open in new window" data-zh="[Mako Next]: A new bundler on top of Turbopack" data-en="[Mako Next]: A new bundler on top of Turbopack">[Mako Next]: A new bundler on top of Turbopack</a> |
There was a problem hiding this comment.
Fixed in 0760e75: the Mako Next aria-label and data-aria-zh now preserve the square brackets, matching the visible/data title.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/aria-label-i18n.test.mjs (1)
1-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid coverage; consider also asserting aria-label mirrors visible title text.
Current test only checks
aria-label === data-aria-zh, not that it matches the anchor's actual visible text/data-zh. That would have caught the bracket-omission mismatch on the Mako Next entry inindex.html.🤖 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 `@tests/aria-label-i18n.test.mjs` around lines 1 - 51, The aria-label coverage in aria-label-i18n.test.mjs only checks against data-aria-zh, so extend the external link assertions to also verify the anchor’s aria-label matches its visible localized title text/data-zh from index.html. Update getExternalLinks or the loop in the test to capture the link text/visible label and compare it with aria-label so mismatches like the Mako Next bracket omission are caught.
🤖 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.
Inline comments:
In `@index.html`:
- Line 234: The anchor in the main HTML content has mismatched accessible text
because the aria-label and data-aria-* values omit the square brackets that are
present in the visible link title and data-zh/data-en strings. Update the link’s
accessibility metadata in the anchor element so aria-label, data-aria-zh, and
data-aria-en exactly match the rendered title “[Mako Next]: A new bundler on top
of Turbopack”, keeping the visible text and all localized variants consistent.
---
Nitpick comments:
In `@tests/aria-label-i18n.test.mjs`:
- Around line 1-51: The aria-label coverage in aria-label-i18n.test.mjs only
checks against data-aria-zh, so extend the external link assertions to also
verify the anchor’s aria-label matches its visible localized title text/data-zh
from index.html. Update getExternalLinks or the loop in the test to capture the
link text/visible label and compare it with aria-label so mismatches like the
Mako Next bracket omission are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dbc2c84a-e65d-472c-9b7f-7998b4eb33df
📒 Files selected for processing (4)
index.htmljs/i18n.jspackage.jsontests/aria-label-i18n.test.mjs
1 participant
Summary
data-aria-zh/data-aria-envalues for external links that open in a new window.aria-labelalongside visible text.Closes #15
Verification
npm testnpx biome lint index.html js/i18n.js tests/aria-label-i18n.test.mjsgit diff --checkSummary by CodeRabbit
testscript to run the Node-based test suite.