Skip to content

fix(ui): repair text spacing left behind by the icon migration (#4217 follow-up)#4238

Merged
Yeraze merged 1 commit into
mainfrom
fix/icon-migration-locale-spacing
Jul 21, 2026
Merged

fix(ui): repair text spacing left behind by the icon migration (#4217 follow-up)#4238
Yeraze merged 1 commit into
mainfrom
fix/icon-migration-locale-spacing

Conversation

@Yeraze

@Yeraze Yeraze commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Follow-up to #4217, which stripped decorative glyphs from translated strings and from one tooltip but did not collapse the whitespace or restore the separators around them.

Changes

Locale double-spacesdashboard.telemetry_help.step4 read "Click the star icon…" in en, fr, pl, ru, zh_Hans after the ⭐ was removed. Collapsed to a single space. zh_Hans collapses to no space, since Chinese does not space between words.

Tooltip separatorsrc/components/map/popups/sections.tsx lost its , leaving the run-on Open <source> Node Details. Rephrased to Open Node Details for <source>.

I chose rephrasing over restoring the arrow deliberately: no-hardcoded-ui-glyph evaluates each template-literal quasi separately, so in `Open ${s.sourceName} → Node Details` the second quasi is " → Node Details" and matches LEADING_UI_SYMBOL_PATTERN (/^\s*[…→…]/). The arrow is mid-sentence in the rendered string, so this trips the rule contrary to its own comment ("Mid-sentence arrows describe direction/relationships; a leading arrow is an icon"). Rephrasing avoids both the awkward string and an eslint-disable for a cosmetic tooltip. The quasi-boundary false positive itself is left unfixed — see below.

Not addressed here

Two items from the #4217 review remain open, both worth a separate issue:

  1. no-hardcoded-ui-glyph's character class omits ●○◐⊘, so four functional status indicators were missed by the migration and can be reintroduced with green CI: MeshCoreRoomsView.tsx:239 (three-state login dot), LiveTracePanel.tsx:135, RadioConfigurationSection.tsx:503-504, ChannelsConfigSection.tsx:548.
  2. The quasi-boundary false positive described above.

Validation

  • npx eslint src/components/map/popups/sections.tsx — clean
  • node scripts/check-ui-locale-glyphs.mjs — passed
  • All 5 edited locale files parse as valid JSON; key sets unchanged
  • vitest run src/components/map/popups — 115 passed, success: true

npm run lint:ci fails locally, but every failing path is under .claude/worktrees/ (stale local agent worktrees, git-excluded). Zero non-worktree ratchet violations.

No behavior change; no locale keys added, renamed, or dropped.

🤖 Generated with Claude Code

https://claude.ai/code/session_016bxVewVnissUKiGZmhf9FW

…follow-up)

The icon-system migration stripped decorative glyphs from translated
strings and from one tooltip, but did not collapse the whitespace or
restore the separators around them.

- dashboard.telemetry_help.step4: the stripped star left a double space
  in en/fr/pl/ru/zh_Hans. Collapsed to a single space; zh_Hans collapses
  to none, since Chinese does not space between words.
- map/popups/sections.tsx: the source-row tooltip lost its "→" separator,
  leaving the run-on "Open <source> Node Details". Rephrased to
  "Open Node Details for <source>" rather than restoring the arrow --
  no-hardcoded-ui-glyph evaluates each template-literal quasi separately,
  so a mid-sentence arrow reads as leading (" → Node Details") and trips
  the rule, contrary to its own mid-sentence-arrow exemption.

No behavior change; locale keys are untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016bxVewVnissUKiGZmhf9FW
@github-actions

github-actions Bot commented Jul 20, 2026

Copy link
Copy Markdown

Claude finished @Yeraze's task —— View job


PR Review: fix(ui): repair text spacing left behind by the icon migration

  • Gather context
  • Read changed files
  • Review locale changes
  • Review sections.tsx change
  • Post final review

This is a clean, surgical follow-up to #4217. The changes are correct and the reasoning in the PR description is sound. A few observations:

Locale fixes (en, pl, ru, zh_Hans)
All four are straightforward double-space → single-space collapses. The diff confirms each file had a trailing space where the emoji was removed (e.g., "Click the star icon…""Click the star icon…").

French locale — minor note 🔍
fr.json previously had "Cliquez sur l'icône à côté de…" (double space after l'icône), which is now collapsed to single space. However, the resulting string "Cliquez sur l'icône à côté de la métrique…" now reads as "Click the icon next to the metric…" — it no longer specifies which icon (the star). The English equivalent says "Click the star icon…". This was an issue before this PR (the emoji was stripped in #4217 without replacing with the word "star"), so this PR isn't regressing anything — but it's worth tracking: the French string is slightly less informative than the English.

sections.tsx tooltip
Open Node Details for <source> is a clear improvement over the prior run-on Open <source> Node Details. The phrasing is idiomatic and the reasoning about avoiding the ESLint quasi-boundary false positive is well-documented. The change is isolated to the title attribute on line 268, no other logic is touched.

zh_Hans
Correct to collapse to no space — Chinese typography doesn't use word spaces, and "…旁边的星标图标" (no space before 星标) is the right behavior.

No issues found — the changes are minimal, correct, and well-tested per the PR description (115 popup tests passing, locale scripts clean).

One optional suggestion for a follow-up (not a blocker): the French string could say "Cliquez sur l'icône étoile à côté de…" to restore parity with English's "star icon" — but that's out of scope for this PR since #4217 introduced that regression, not this one.

Verdict: approve as-is.


@Yeraze
Yeraze merged commit df4098a into main Jul 21, 2026
19 checks passed
@Yeraze
Yeraze deleted the fix/icon-migration-locale-spacing branch July 21, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant