Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

plugin-dashboard: WidgetConfigPanel's private resolveLabel reads only the RETIRED label form — an inline-locale-map title/description opens the panel EMPTY and can be saved over #5301

Closed

Description

@os-support-ai

Found while implementing #5264 (PR migrating ObjectMetricWidgetProps to string | I18nLabel). Not fixed there — different member surface, and the fix needs a decision this card records rather than guesses.

What

packages/plugin-dashboard/src/WidgetConfigPanel.tsx carries a private label resolver:

/** Resolve an I18nLabel (string or {key, defaultValue}) to a plain string. */
function resolveLabel(v: unknown): string {
  if (v === undefined || v === null) return '';
  if (typeof v === 'string') return v;
  if (typeof v === 'object') {
    const obj = v as Record<string, any>;
    return obj.defaultValue || obj.key || '';
  }
  return String(v);
}

Its doc comment claims it resolves an I18nLabel. It does the opposite: { key, defaultValue } is the key-reference form @objectstack/spec RETIRED at 17.0.0-rc.6 (objectstack#5055), and the inline per-locale map that I18nLabelSchema actually admits has neither limb. Measured against this tree:

input resolveLabel returns
{ en: 'Revenue', 'zh-CN': ... } — the spec's vocabulary ""
{ key: 'a.b.c', defaultValue: 'Revenue' } — retired "Revenue"
'Revenue' "Revenue"

It is the same private-resolver class objectui#4032 swept out of DashboardRenderer, MetricWidget and MetricCard; this fourth copy was not in that pass. It is also the mirror of #5134 — a declared I18nLabel surface whose only read site cannot read the form the spec admits.

Why it is not merely cosmetic

The resolved value is not display-only — it seeds the editable draft:

const normalizedConfig = React.useMemo(() => ({
  ...config,
  title: typeof config.title === 'object' ? resolveLabel(config.title) : config.title,
  description: typeof config.description === 'object' ? resolveLabel(config.description) : config.description,
}), [config]);

const { draft, isDirty, updateField, discard } = useConfigDraft(normalizedConfig, ...);

So for a widget whose stored title is an inline locale map, opening the config panel shows an EMPTY Title field, and the draft that a save writes back carries '' where the author's map used to be. The panel is the surface Studio authors dashboards through, so the destructive path is the ordinary one — open the widget, change anything else, save.

Why it was not folded into #5264

That PR's fix shape is binding and scoped to ObjectMetricWidgetProps plus the fixture sweep. This is a different member surface (DashboardWidget config, not the widget's own props), and — unlike the four members there — the fix is not mechanical: resolveLabel is a module-level function with no locale in scope, so making it locale-aware means deciding which locale an authoring panel should display, and what a save should then write back. Those are not the same question:

  • Reading: pickLocalized(value, language) matches every sibling surface.
  • Writing: collapsing a multi-locale map to the one string the editor showed silently discards the other locales. Preserving them needs the panel to edit a map, or to write back only the active locale's entry.

The second half is a real design decision (arguably a Studio authoring-UX question, cf. #4163's "give Studio a way to author one"), which is why this is filed rather than guessed.

Repro

resolveLabel({ en: 'Revenue', 'zh-CN': '收入' })  // => ""  (expected "Revenue" / "收入")

Note the pure-function repro is the honest one here: the panel path reaches it through normalizedConfig, so a component-level test would also be asserting useConfigDraft's behaviour.

Activity

  1. added theissue type on Aug 19, 2026
  2. os-warren commented on Aug 19, 2026

    @os-warren
    Collaborator

    Triage: type Bug, routed to the decision inbox (needs-user-decision) rather than the queue — the read half is mechanical (pickLocalized(value, language), matching every sibling surface post-#4032), but what a SAVE writes back to a multi-locale label is an authoring-UX/product call the card correctly refuses to guess.

    • platform long-term coherence: fourth private copy of the retired-form resolver (objectui: dashboard KPI cards drop the translated widget title/description — the self-contained metric path bypasses tWidgetTitle, and the plugin's private resolveLabel never calls t() #4032 swept three); whichever write-back rule is chosen should be stated once and shared, not re-derived per panel.
    • measured business pull: the destructive path is the ordinary one — open a widget whose title is an inline locale map, change anything, save ⇒ the author's map is replaced by ''. Studio authors hit this on every map-titled widget today.
    • AI-agent error-resistance: an editor that silently collapses a multi-locale map to one string (or to empty) destroys data with no failure signal; write-back-only-the-edited-locale keeps saves idempotent for untouched fields.
    • startup scope discipline: a full map-editing UI is the expensive road; the minimal honest fix (display via pickLocalized; on save, write the edited string into the active locale's entry and preserve the rest; untouched fields round-trip unchanged) needs no new UI.

    Recommendation: A — display via pickLocalized(value, language); an un-edited title/description round-trips the original value byte-for-byte; an edited one writes the new string into the active locale's map entry, preserving other locales. Map-editing UX (authoring all locales in the panel, cf. #4163) stays a separate future card. B — same read fix, but an edit collapses the map to a plain string (simplest, lossy, loud in review). C — full multi-locale editing UI now.

    Confidence gap: this analysis has not checked what the dashboard SAVE path persists for other object-valued config keys — if the panel already round-trips unknown keys untouched, A is smaller than it looks; if it rebuilds the whole config from the draft, "preserve untouched fields" needs a real diff mechanism.


    Generated by Claude Code

  3. removed their assignment
    on Aug 20, 2026
  4. os-zhuang commented on Aug 20, 2026

    @os-zhuang
    Contributor

    Maintainer ruling recorded (2026-08-20, live decision-inbox session with the triage seat, session session_01PjAP6vbcsg2yMtvySPv1Qo)

    Ruled. Provenance: maintainer accepted the batch, verbatim: 「其他接受你的建议。」

    State transition in the same stroke: needs-user-decision → pm:queue.


    Generated by Claude Code

  5. os-support-ai commented on Aug 20, 2026

    @os-support-ai
    CollaboratorAuthor

    Claim: PM loop round 21
    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-5301-widget-config-panel-locale-map
    Worktree: objectui-issue-5301
    Domain: repo:objectui
    File surface: packages/plugin-dashboard/src/WidgetConfigPanel.tsx and whatever its save path turns out to be (useConfigDraft and its consumers) (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Clause-②: no — makes a declared I18nLabel surface readable in the form the spec admits; no contract change
    Serial constraints cleared: packages/plugin-dashboard is disjoint from the other three this round (#5240 plugin-grid, #3318 packages/fields, #5389 packages/auth). ⚠️ #5264 previously touched this package's ObjectMetricWidgetProps — that PR is merged, and this card is a different member surface (DashboardWidget config, not the widget's own props). No open PR touches plugin-dashboard.

    Dispatched on the maintainer's 2026-08-20 ruling above (「其他接受你的建议。」): read via pickLocalized(value, language); a save writes back only the active locale's entry, preserving the other locales — ⛔ never collapsing the map to the one string the editor showed.

    ⚠️ The dispatch order carries triage's own confidence gap as a required first measurement: nobody has checked what the dashboard save path persists for other object-valued config keys. If the panel already round-trips untouched keys, the fix is small; if it rebuilds the whole config from the draft, "preserve untouched fields" needs a real diff mechanism and the card is bigger than it reads.


    Generated by Claude Code

  6. os-support-ai commented on Aug 20, 2026

    @os-support-ai
    CollaboratorAuthor

    os-dev-report

    {
      "issue": 5301,
      "status": "done",
      "branch": "claude/issue-5301-widget-config-panel-locale-map",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5427",
      "premise_still_valid": true,
      "summary": "MEASURED FIRST, per the dispatch's flagged confidence gap: the panel already round-trips untouched keys and does NOT rebuild the config from the draft, so no diff mechanism is needed and this card is the small shape. Chain: useConfigDraft seeds useState({...source}) (a shallow copy of the WHOLE config, not a projection over the schema's field list); updateField returns {...prev, [field]: value}; sanitizeDraftForType spreads the draft and deletes a fixed deny-list; DashboardWithConfig.handleWidgetSave forwards it verbatim. Object-valued keys already survive (dimensions/values are arrays carried across untouched, pinned by the existing sanitize tests). The title/description normalization was the SINGLE lossy step. Fix per the 2026-08-20 ruling: reading via pickLocalized(value, language); a save writes back only the active locale's entry, with an untouched label restored as the stored object BY REFERENCE (asserted toBe, not toEqual - a rebuild would add an entry for the active locale to a map that never carried one). onFieldChange is fixed too: it is a second data-loss path, since DashboardWithConfig writes it into liveSchema which is where the next config prop is derived from, so a bare string there dropped the map before a save ever ran. New shared helper setLocalized(value, language, next) in @object-ui/i18n, co-located with pickLocalized because the entry written must be the entry read; it follows pickLocalized's first three limbs (exact tag, base, regional sibling) and deliberately stops there, since default/en/first-value are DISPLAY fallbacks and writing to one would let an author editing in fr overwrite English. A full multi-locale editor stays out of scope.",
      "tests": "All from the repo root at tree 369e6a073 (= PR head, = the tree every number below was measured on). Vitest resolves @object-ui/i18n to packages/i18n/src via the root config alias, so NO build artifact sits between the edit and the thing under test on any test leg; the type-check leg is different (tsc has no paths entry for @object-ui/i18n and resolves it to dist), so the dependency closure was built first via pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-dashboard^...' build, and setLocalized was proven present in packages/i18n/dist/index.d.ts before type-checking. GREEN: vitest run packages/plugin-dashboard packages/i18n => 'Test Files 115 passed (115) / Tests 1496 passed (1496)'. Post-ablation re-verify at HEAD => 'Test Files 6 passed (6) / Tests 120 passed (120)'. type-check (plugin-dashboard + i18n) => both echoed the script name and printed Done, PIPE_EXIT=0 (not a zero-match silent pass). lint => '329 problems (0 errors, 329 warnings)', all pre-existing any/fast-refresh. Gates: check:control-bytes OK (4485 files); check:self-import 0; check:phantom-deps OK; check:i18n-keys OK (2904 keys); check:i18n-drift 0 en values changed; check-changeset-presence/no-major/fixed all OK. Plus a manual control-byte grep over all six changed files: no matches. NEW COVERAGE: 29 cases in packages/i18n/src/__tests__/setLocalized.test.ts, 21 in packages/plugin-dashboard/src/__tests__/WidgetConfigPanel.inlineLocaleMap.test.tsx. REVERSE VERIFICATION, two legs, predicted before running, both run from the COMMITTED fix and restored with git checkout HEAD -- (marker absence verified after each). Neither leg needed a rebuild (source-resolved, per the alias above); both legs were mutated and re-run in place. LEG A (read half reverted to the retired defaultValue||key resolver): predicted 6 red - 3 display cases, 2 untouched-round-trip cases, the de case's display assertion. OBSERVED 7. The prediction MISSED one and the miss is informative rather than cosmetic: 'writes an emptied field into the active locale' types '' into a field the ablated read had already rendered as '', so React dispatches no change event, the panel never goes dirty, and the Save button (rendered only when dirty) is never found - it fails mechanically, not on its assertion. The other six failed with the defect verbatim: \"expected '' to be '收入'\" and \"expected { en: 'Revenue', zh: '' } to be { en: 'Revenue', zh: '收入' }\". setLocalized.test.ts stayed fully green, as predicted. LEG B (write-back removed from the save path, read intact): predicted 6 red, every save-payload case, display and onFieldChange staying green. OBSERVED EXACTLY 6, messages showing the data-loss shape directly: \"expected '收入' to be { en: 'Revenue', zh: '收入' }\" and \"expected 'Umsatz' to deeply equal { en: 'Revenue', zh: '收入', ...(1) }\".",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5428: plugin-designer's DashboardEditor keeps a map-valued widget title read-only on a premise the spec invalidated - isAuthorableTitle's doc says 'no persisted widget title can be a map' while resolveWidgetTitle's doc 60 lines above in the SAME file says rc.6 widened I18nLabel so it can. The read-only branch itself cannot lose data (observation, not defect), but the card it defers to for the real answer, #4163, was closed as completed on 2026-08-15 while the placeholder is still in the tree, so the deferral now points at nothing. Newly actionable because this card's ruling plus setLocalized supply exactly the rule that placeholder was waiting for. Filed unassigned, label 'finding', not queued."
      ]
    }

    Generated by Claude Code

  7. os-support-ai commented on Aug 20, 2026

    @os-support-ai
    CollaboratorAuthor

    ACCEPT — PR #5427 enqueued

    repo:objectui execution seat, session session_01RV6yuVCxymHYE16PL9vQkE. All 21 check runs completed: 19 success, 2 skipped. Shards 6m — 9m14s, Type Check 6m48s, Lint 5m04s. Marked ready, then auto-merge (SQUASH).

    The precondition I set was measured, and it collapsed the card to the small shape

    The dispatch made triage's flagged confidence gap a required first step: nobody had checked what the dashboard save path persists for other object-valued config keys, and if the panel rebuilt the config from the draft, "preserve untouched fields" would have needed a real diff mechanism.

    Measured: it already round-trips untouched keys. The chain, traced end to end — useConfigDraft seeds useState({...source}), a shallow copy of the whole config rather than a projection over the schema's field list; updateField returns {...prev, [field]: value}; sanitizeDraftForType spreads and deletes a fixed deny-list; handleWidgetSave forwards verbatim. Object-valued keys already survive (dimensions/values are arrays carried across untouched, pinned by existing tests). So title/description normalisation was the single lossy step, which is exactly where the fix lands. No diff mechanism, no fork.

    A second data-loss path the card did not name

    onFieldChange — the live-update callback. DashboardWithConfig writes it into liveSchema, which is where the next config prop is derived from, so forwarding the bare editor string dropped the map before any save ran: open, type, close without saving, re-open, and the other locales were already gone. Found and fixed, not deferred.

    Two judgements worth keeping

    • toBe, not toEqual, for the untouched round-trip. The reasoning: an equal-but-rebuilt object is a weaker guarantee, because a rebuild would add an entry for the active locale to a map that never carried one. Identity is the property that actually encodes "untouched".
    • setLocalized deliberately stops at three limbs — exact tag, base language, region-qualified sibling — and ⛔ not default / en / first-value. Those are display fallbacks that hand back another locale's string; writing through one would let an author editing in fr silently overwrite English. That is a data-loss trap avoided by construction rather than by test.
    • language is a memo dependency with a stated reason: omitting it would let a mid-session language switch compare an old-locale draft against the new locale, so every untouched label would read as edited and merge into the wrong entry.

    Reverse verification

    Leg A (read reverted): predicted 6 red, observed 7 — and the extra is explained rather than smoothed over. "writes an emptied field into the active locale" types '' into a field the ablated read had already rendered as '', so React dispatches no change event, the panel never goes dirty, and the Save button — rendered only when dirty — is never found. It fails mechanically, not on its assertion, which is why the prediction missed it. Leg B (write-back removed): predicted 6, observed exactly 6, with messages showing the data-loss shape directly.

    Two things recorded, neither blocking

    Card closes on merge via Fixes #5301.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions