Repository navigation
fix(web): give the English flag icon unique clipPath ids (#1079) - #1080
Conversation
The navbar renders the locale switcher twice, so the fixed flag-en-s and flag-en-t clipPath ids were duplicated on every English page. Derive them from useId per instance. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
If your organization's extra usage balance is empty, an organization admin can add extra usage credits at claude.ai/admin-settings/usage. If its monthly spend limit was reached, an admin can raise it on the same page. If neither applies, contact Anthropic support.
Once extra usage is available, someone with write access to this repository can comment @claude review on this pull request to trigger a review.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Generate and validate unique flag IDs apps/web/src/components/elementary/LocaleSwitcher.tsx, apps/web/e2e/unique-ids.spec.ts |
FlagEN uses useId for clip-path IDs and their SVG references. The Playwright test checks both English routes for successful responses, visible navigation, and duplicate document IDs. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Suggested labels: area:frontend
Suggested reviewers: dos
Merge Risk: 
6d73b
-
No actionable merge-blocking defect is established.
Pre-merge checks: 
-
Check name Status Explanation Title check The title clearly and concisely describes the main change: assigning unique clipPath IDs to the English flag icon. Description check The description explains what changed, why it changed, affected files, test coverage, regression validation, and change type. It does not include the template checklist or Notes section, but the core … Linked Issues check Issue #1079has two coding requirements. (1) Unique per-instance clipPath ids for the English flag, derived with useId. The head diff of apps/web/src/components/elementary/LocaleSwitcher.tsx adds useI…Out of Scope Changes check The whole-PR diff against the merge base changes two files. LocaleSwitcher.tsx changes only the FlagEN component and the React import. The markup, viewBox, styles, and paths are unchanged, and only th… Docstring Coverage No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2… Full details: Description check
-
Explanation
The description explains what changed, why it changed, affected files, test coverage, regression validation, and change type. It does not include the template checklist or Notes section, but the core information is complete.
Full details: Linked Issues check
-
Explanation
Issue
#1079has two coding requirements. (1) Unique per-instance clipPath ids for the English flag, derived with useId. The head diff of apps/web/src/components/elementary/LocaleSwitcher.tsx adds useId to the React import. FlagEN now calls const clipId = useId() and sets clipPath ids to${clipId}sand${clipId}t. Both clipPath references (clipPath={url(#${clipId}s)} and clipPath={url(#${clipId}t)}) use the same generated values. The fixed flag-en-s and flag-en-t ids are gone. (2) An E2E check that asserts no duplicate ids on the home page and /smartphones and fails without the fix. apps/web/e2e/unique-ids.spec.ts visits /en and /en/smartphones. The route tree shows that [locale]/page.tsx is the home page, so /en is the English home page. The test scans all [id] elements and fails on any duplicate. Without the fix, FlagEN renders the fixed ids twice on /en pages, so the test would fail. The PR narrows the reported duplicates to /en/* pages, which matches the code path (FlagEN renders as the current-locale flag only on English pages or in an open dropdown). The useId fix applies to every instance, so it does not depend on that narrowing. The author's mutation check was not re-run in this review. The assessment relies on code reading.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff against the merge base changes two files. LocaleSwitcher.tsx changes only the FlagEN component and the React import. The markup, viewBox, styles, and paths are unchanged, and only the id values and their url() references change. The new unique-ids.spec.ts is a Playwright test for the duplicate-id requirement in
#1079. Both changes connect directly to the linked issue. No unrelated files or behavior changes appear in the summary or the diffs read.
Full details: Docstring Coverage
-
Explanation
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Closes #1079
What
LocaleSwitcheris rendered twice in the navbar (phone and desktop). On English pages the current-locale flag is the English SVG, whose<clipPath>ids were hard-coded (flag-en-s,flag-en-t), so both ids appeared twice and each copy'surl(#...)resolved to the first one in the document.FlagENnow derives the ids fromuseId.Scope note: the duplicates appear on
/en/*pages (the English flag is only drawn as the current-locale flag there, or in an open dropdown), not on Vietnamese pages as the issue first said.Test
e2e/unique-ids.spec.ts: no duplicate ids on/enand/en/smartphones.LocaleSwitcher.tsxreverted toorigin/dev, both tests fail listingflag-en-sandflag-en-t; with the fix, both pass.🤖 Generated with Claude Code
📌 TL;DR
This PR fixes a DOM accessibility and rendering bug where the
LocaleSwitchercomponent rendered duplicate SVGclipPathIDs, causing visual glitches when the component appeared multiple times on a page. It introduces React'suseIdhook to generate unique identifiers for the English flag SVG and adds an E2E test to prevent future ID collisions.🎯 Type of Change
🔍 Changes Walkthrough
apps/web/src/components/elementary/LocaleSwitcher.tsxFlagENcomponent to use React'suseIdhook. This replaces hardcodedclipPathIDs (flag-en-s,flag-en-t) with dynamic, unique IDs, ensuring that multiple instances of the flag on the same page do not conflict or reference the wrong SVG paths.apps/web/e2e/unique-ids.spec.ts/enand/en/smartphones, scans the DOM for any duplicateidattributes, and asserts that no duplicates exist, specifically guarding against the regression described in #1079.📊 Architectural Flow
sequenceDiagram participant Browser participant React participant DOM Note over Browser,DOM: Page Load (/en) Browser->>React: Render LocaleSwitcher (Mobile) React->>React: Call useId() -> "r1" React->>DOM: Insert SVG with clipPath id="r1s" Browser->>React: Render LocaleSwitcher (Desktop) React->>React: Call useId() -> "r2" React->>DOM: Insert SVG with clipPath id="r2s" Note over DOM: No duplicate IDs DOM-->>Browser: Render complete Note over Browser: E2E Test Execution Browser->>DOM: Query all [id] elements DOM-->>Browser: Return list of IDs Browser->>Browser: Check for duplicates Browser-->>Browser: Assert duplicates == []Summary by CodeRabbit