Skip to content

fix(chore): normalize CJK punctuation and some half/full-width char visual issue - #467

Merged
lucas77778 merged 1 commit into
masterfrom
yihong/code-606
Aug 22, 2026
Merged

fix(chore): normalize CJK punctuation and some half/full-width char visual issue#467
lucas77778 merged 1 commit into
masterfrom
yihong/code-606

Conversation

@xiaoland

@xiaoland xiaoland commented Aug 21, 2026

Copy link
Copy Markdown
Member

Summary

Fixes two issues:

  • The settings.providers section of zh-cn.ts used half-width punctuation (commas, semicolons, question mark, parentheses, ellipsis) inconsistent with the rest of the file. en.ts had one ... instead of .
  • Additionally, the provider picker catalog group labels in add-flow.tsx applied tracking-widest (0.1em letter-spacing) unconditionally. Combined with IBM Plex Sans lacking CJK glyphs (fallback to PingFang), this inflated Latin characters beside Chinese text — making "API" in "API 直连" visually read as full-width.

Solution:

  • For first issue:
    • zh-cn.ts: 6 half-width commas → , 3 semicolons → , 1 question mark → , 5 half-width parentheses → () or spaced half-width for Latin-only brand names (xAI (Grok)), 1 ...
    • en.ts: Search settings...Search settings…
  • For second issue:
    • add-flow.tsx: useLocale() detects zh locales and skips tracking-widest on group labels; English retains the original styling
    • add-flow.test.tsx: added useLocale mock

Closes CODE-606 .

Something else

  • .claude/rules/frontend.md has been updated, aiming to avoid similar problems happens again in the future (though, it's only guidance, not a guardrail like lint rules)

Verification

image image

Copilot AI lite review requested due to automatic review settings August 21, 2026 12:06
@linear-code

linear-code Bot commented Aug 21, 2026

Copy link
Copy Markdown

CODE-606

@xiaoland xiaoland changed the title fix(i18n): normalize CJK punctuation and fix tracking-widest on zh-CN group labels fix(chore): normalize CJK punctuation and some half/full-width char visual issue Aug 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves i18n typography consistency by normalizing punctuation in locale strings (especially for zh-CN) and adjusting UI letter-spacing so mixed Latin/CJK labels render correctly in the provider add flow.

Changes:

  • Normalized zh-CN locale punctuation in settings.providers (full-width punctuation, proper ellipsis, and spacing for Latin-only brand names).
  • Replaced a three-dot ellipsis with in the English settings search placeholder.
  • Skipped tracking-widest for provider catalog group labels under zh locales to avoid inflated Latin glyph spacing; updated the related test mock.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/presentation/i18n/src/locales/zh-cn.ts Normalizes CJK punctuation and spacing in provider/settings-related zh-CN strings.
packages/presentation/i18n/src/locales/en.ts Switches settings search placeholder to use the single ellipsis character ().
packages/client/workbench/src/settings/providers/add-flow.tsx Uses useLocale() to conditionally omit tracking-widest for zh locales on group labels.
packages/client/workbench/src/settings/providers/tests/add-flow.test.tsx Updates use-intl mock to include useLocale() for the new hook usage.
.claude/rules/frontend.md Documents the CJK typography convention to prevent future locale-string regressions.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No bugs found — one style nit inline, plus two scope questions worth a decision before merge.

Reviewed changes — the full diff at 582138ff: the zh-cn.ts punctuation sweep, the one en.ts ellipsis, the locale-conditional tracking-widest fix, its test mock, and the new frontend.md convention.

  • zh-cn.ts punctuation normalization — half-width ,;? and () inside settings.agents / settings.providers become full-width, and ... becomes . I verified the sweep is complete: zero remaining half-width ,;?!() adjacent to a CJK codepoint anywhere in the file.
  • Latin brand names get spacing instead of full-width parensxAI (Grok) and Cloudflare AI Gateway (Anthropic) now match en.ts:1124 / en.ts:1131 character-for-character, so this is a genuine cross-locale consistency fix rather than a stylistic preference.
  • en.ts:786 ellipsis'Search settings...''Search settings…'.
  • add-flow.tsx group labelsServiceCatalogView reads useLocale() and drops tracking-widest for zh locales. The justification checks out: the zh label for the direct group is literally API 直连, so the Latin run really does sit beside CJK glyphs.
  • add-flow.test.tsx:15 — adds useLocale: () => 'en' to the existing use-intl mock. No other test breaks: providers-settings.test.tsx:33 mocks the whole ../add-flow module, so its useLocale-less mock never reaches the new call.
  • frontend.md:42 — documents the CJK typography convention for locale strings.

I also confirmed nothing depends on the old literal text. There are no project snapshots, the use-intl test mocks are key-pass-through (tests match i18n keys, not values), and although providers.serviceName.xai is a settings-search keyword via useSettingsSearchKeywords, matchPaletteCommands scores by lowercased substring/token containment, so the added space is behaviorally inert.

ℹ️ The new convention documents the string half but not the styling half, and one live sibling still carries the pattern

frontend.md:42 captures the punctuation rules well, but the insight that produced the add-flow.tsx change — don't apply a tracking-* utility to an element rendering translated labels without a zh escape hatch — isn't written down anywhere. That's the half a future agent will re-break, since the fix now lives as an unexplained local variable in one component.

The one live sibling with the same shape is account-detail.tsx:149 and :207, in this same providers subsystem. To be precise about severity: both render pure-CJK labels (凭证, 接入的智能体) with no Latin run, so the specific inflation symptom this PR fixes does not occur there — tracking-wide on ideographs is just uniform inter-character space, which is defensible Chinese typography. So this is a scope question, not a missed bug: is the rule "no letter-spacing on any label that can render CJK", or narrowly "no letter-spacing where Latin and CJK mix"?

Technical details
# Decide the scope of the letter-spacing rule, then write it down

## Affected sites
- `.claude/rules/frontend.md:42` — the new bullet covers punctuation in locale strings only; the styling half of this PR is undocumented.
- `packages/client/workbench/src/settings/providers/add-flow.tsx:141-143` — the reasoning currently lives only in a code comment on the one site that was fixed.
- `packages/presentation/ui/src/shell/providers/account-detail.tsx:149``uppercase tracking-wide` over `{t('credential')}` (`凭证` in zh-CN). Live via `providers-settings.tsx:220`.
- `packages/presentation/ui/src/shell/providers/account-detail.tsx:207` — same, over `{t('connections')}` (`接入的智能体`).

## Required outcome
- A decision on which rule is intended, recorded in `frontend.md` next to the punctuation bullet so it survives beyond this PR.
- If the rule is the broad one, `account-detail.tsx` is brought in line. If it is the narrow one, say so explicitly — the narrow rule is a defensible read and the two `account-detail` labels are then correct as-is.

## Open questions for the human
- Does the rule key on "the label can contain CJK" or on "the label mixes CJK with a Latin run"?
- `ToolSection` (`packages/presentation/ui/src/chat/tool.tsx:204`) and `FilterSidebarGroup` (`apps/webview/src/components/filter-sidebar-layout.tsx:82`) both apply `uppercase tracking-wide|wider` to a `label` prop and currently have zero callers. Per AGENTS.md ("Restructure, Don't Just Remove") these are deliberate scaffolding, not dead code — should whichever rule you pick be applied to them pre-emptively, or left for whoever wires them up?

ℹ️ <html lang> is hard-coded to zh-CN and never synced to the resolved locale

Both renderer entries ship lang="zh-CN", and nothing in the app ever writes document.documentElement.lang — a grep for documentElement turns up only theme, appearance, and density writes. This is pre-existing and not something this PR caused, but it sits squarely in the domain the PR is fixing, and it's worth knowing for two reasons.

First, it means the CSS-native fix for this exact defect class (a :lang() rule or a Tailwind lang-* variant, applied once instead of per-component) is currently unusable — which retroactively justifies reaching for useLocale() here rather than making it look like the wrong tool. Second, English users are running under a Chinese language context, so the user agent applies CJK font-selection and line-breaking heuristics to English text. Fixing the sync would unlock the cheaper mechanism for the next label that needs this treatment.

Technical details
# `documentElement.lang` never reflects the resolved locale

## Affected sites
- `apps/desktop/src/renderer/index.html:2``<html lang="zh-CN" class="dark">`, static.
- `apps/webview/index.html:2``<html lang="zh-CN">`, static.
- `packages/client/workbench/src/app/app-providers.tsx:26-33``AppI18nProvider` resolves the locale and hands it to `IntlProvider`, but never propagates it to the document element.

## Required outcome
- `document.documentElement.lang` reflects the locale that `AppI18nProvider` actually resolved, so UA font-selection and line-breaking match the rendered language.
- Once it does, a locale-conditional typographic rule can be expressed once in CSS rather than re-derived per component via `useLocale()`.

## Suggested approach (optional)
- The existing precedent is `packages/client/workbench/src/settings/appearance-effects.ts`, which already writes `documentElement` class/dataset/style from resolved state — the same seam could own `lang`.
- Out of scope for this PR; worth its own issue rather than expanding this one.

ℹ️ Nitpicks

  • Nothing pins the zh branch of the new logic. The added useLocale: () => 'en' mock only keeps the existing suite green, so the fix has no regression guard. A single render with the mock returning 'zh-CN' asserting the group label's class list lacks tracking-widest would fail with the bug present, which makes it real coverage rather than theatre — though it does couple a test to a Tailwind class name, so your call whether that trade is worth it.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/client/workbench/src/settings/providers/add-flow.tsx
… group labels

zh-cn.ts settings.providers section used half-width punctuation (commas,
semicolons, question mark, parentheses, ellipsis) inconsistent with the
rest of the file. en.ts had one ... instead of ….

add-flow.tsx group labels applied tracking-widest unconditionally; the
0.1em letter-spacing inflates Latin glyphs beside CJK fallback fonts,
making 'API' in 'API 直连' visually read as full-width. Skip
tracking-widest for zh locales via useLocale().

Add CJK typography convention to .claude/rules/frontend.md.
@lucas77778
lucas77778 self-requested a review August 22, 2026 07:02

@lucas77778 lucas77778 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lucas77778
lucas77778 merged commit 63b3813 into master Aug 22, 2026
12 checks passed
@lucas77778
lucas77778 deleted the yihong/code-606 branch August 22, 2026 07:03
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.

3 participants