fix(i18n): confirm bodies lead with consequence, one discard verb (#5243) - #5529
Conversation
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
Design Review (Fable 5) — ✅ PASSDesign-level review of Design-Verdict: PASS Copy-only fix with a real UX defect, values-only across all 13 catalogs, gate-verified, and the acknowledged ru/ko drift correctly deferred rather than scope-crept. [DESIGN-REVIEWED] 7e00c17 |
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSUX-level review of UX-Verdict: PASS Consequence-first bodies, unified "changes" verb, and confirm buttons that restate the action — the screenshots match the strings exactly across all 12 locales. Suggestions
[UX-REVIEWED] 7e00c17 |
First Principles Review (Fable 5) — ✅ PASSPremise-level review of Reading done — patch, intent, contract, and the surrounding repo (confirm call sites, catalogs, capture conventions). Verified counts: the 4 rewritten bodies are the only First-Principles-Verdict: PASS Fixes the two reported copy defects at their cause — the strings themselves — covering all 4 affected bodies in all 13 catalogs, with nothing riding along. What this change shipsIntent: make the migrated confirm dialogs read correctly (body states the consequence once; discard uses one verb) — a FIX.
Coverage check that supports PASS: grepped [FIRST-PRINCIPLES-REVIEWED] 7e00c17 |
d7a9944 to
96fa4c0
Compare
) Follow-up to the #699 keeper-half migration (#5242), which moved window.confirm() call sites into the themed confirm dialog but froze all user-facing strings. Two copy defects remained, both flagged by the UX review on #5242: 1. Bodies restated their titles: the confirm bodies were written to stand alone in an OS sheet, so under a dialog title they asked the question twice — the second time in caps ("Destroy site?" over "DESTROY 'blog'? ..."). The four bodies (three pages.artifactDeployPage.*_confirm, one apps.papyrus.workspace.co_author_conflict_discard_confirm) now open with the consequence and the resource name; the title keeps the question. 2. The discard verb varied across discard-guard dialogs: Papyrus said "edits" while MarkdownPanel/ArtifactDetailPage say "changes". Unified on "changes" (the majority term); the Papyrus trigger keeps its "and reload" consequence wording. Each catalog's Papyrus discard term now matches that catalog's existing discard_unsaved_changes term. Values only — no key renames, no component or behavior change. The same edits are swept across all 11 translated catalogs (locale quote pairs and French U+202F narrow no-break spaces preserved; the {{file}} operand stays quoted per destructiveConfirm.test.ts QUOTED_OPERAND_CONFIRM_KEYS) and en-XA is regenerated via scripts/gen-pseudolocale.mjs. PapyrusPageCoverage selectors updated to the new copy (the failing selectors double as regression pins). Evidence harness capture/confirm-dialog-copy.* photographs the REAL useConfirm() dialog and asserts the rendered text before writing each frame. Closes #5243
96fa4c0 to
7e00c17
Compare
bolichen97
left a comment
There was a problem hiding this comment.
Approving per triage sweep: readiness passed, no blocking reviews, fix-type change. Auto-merge will be enabled; branch protection still gates.
A chat folder already carries a project_dir and nests via parent_id, but a monorepo or a directory of sibling repositories needs one folder per package before per-package steering loads at all — and assembling N sub-folders by hand is work nobody does. This adds a pure read-only scanner (project_scan.py) that detects packages via repository/manifest/deploy-root/workspace-member signals with two confidence tiers, honoring the project's own .gitignore with git semantics; two endpoints (scan = dry-run preview, scaffold = create the confirmed selection through the existing folder create path); and a manifest-only builtin app providing the preview/confirm UI. Detailed design notes live in docs/system-specs. Two touches fall outside the feature's own files, both required to keep house gates green: - website/scripts/check-bundle-size.mjs raises the 'all' chunk budget 9200 -> 9300 KB. That chunk is the eager i18n catalogs; this app's 37 keys across 13 locales add 105 KB and the entry's own comment scopes the ceiling to catching a new library, not translated strings. The app's page is behind a lazy import() and lands in its own chunk. - website/src/i18n/locales/hi.json restores one Papyrus string to the informal register hi.md §4 requires. It regressed to the formal आप in d805995 (kirodotdev#5529), which took main to 120 against its own ceiling of 119, so hiStyle fails on main independently of this branch. Fixing the word puts it back at 119 rather than loosening the ratchet. Closes kirodotdev#2516
A chat folder already carries a project_dir and nests via parent_id, but a monorepo or a directory of sibling repositories needs one folder per package before per-package steering loads at all — and assembling N sub-folders by hand is work nobody does. This adds a pure read-only scanner (project_scan.py) that detects packages via repository/manifest/deploy-root/workspace-member signals with two confidence tiers, honoring the project's own .gitignore with git semantics; two endpoints (scan = dry-run preview, scaffold = create the confirmed selection through the existing folder create path); and a manifest-only builtin app providing the preview/confirm UI. Detailed design notes live in docs/system-specs. Two touches fall outside the feature's own files, both required to keep house gates green: - website/scripts/check-bundle-size.mjs raises the 'all' chunk budget 9200 -> 9300 KB. That chunk is the eager i18n catalogs; this app's 37 keys across 13 locales add 105 KB and the entry's own comment scopes the ceiling to catching a new library, not translated strings. The app's page is behind a lazy import() and lands in its own chunk. - website/src/i18n/locales/hi.json restores one Papyrus string to the informal register hi.md §4 requires. It regressed to the formal आप in d805995 (kirodotdev#5529), which took main to 120 against its own ceiling of 119, so hiStyle fails on main independently of this branch. Fixing the word puts it back at 119 rather than loosening the ratchet. Closes kirodotdev#2516
A chat folder already carries a project_dir and nests via parent_id, but a monorepo or a directory of sibling repositories needs one folder per package before per-package steering loads at all — and assembling N sub-folders by hand is work nobody does. This adds a pure read-only scanner (project_scan.py) that detects packages via repository/manifest/deploy-root/workspace-member signals with two confidence tiers, honoring the project's own .gitignore with git semantics; two endpoints (scan = dry-run preview, scaffold = create the confirmed selection through the existing folder create path); and a manifest-only builtin app providing the preview/confirm UI. Detailed design notes live in docs/system-specs. Two touches fall outside the feature's own files, both required to keep house gates green: - website/scripts/check-bundle-size.mjs raises the 'all' chunk budget 9200 -> 9300 KB. That chunk is the eager i18n catalogs; this app's 37 keys across 13 locales add 105 KB and the entry's own comment scopes the ceiling to catching a new library, not translated strings. The app's page is behind a lazy import() and lands in its own chunk. - website/src/i18n/locales/hi.json restores one Papyrus string to the informal register hi.md §4 requires. It regressed to the formal आप in d805995 (kirodotdev#5529), which took main to 120 against its own ceiling of 119, so hiStyle fails on main independently of this branch. Fixing the word puts it back at 119 rather than loosening the ratchet. Closes kirodotdev#2516
A chat folder already carries a project_dir and nests via parent_id, but a monorepo or a directory of sibling repositories needs one folder per package before per-package steering loads at all — and assembling N sub-folders by hand is work nobody does. This adds a pure read-only scanner (project_scan.py) that detects packages via repository/manifest/deploy-root/workspace-member signals with two confidence tiers, honoring the project's own .gitignore with git semantics; two endpoints (scan = dry-run preview, scaffold = create the confirmed selection through the existing folder create path); and a manifest-only builtin app providing the preview/confirm UI. Detailed design notes live in docs/system-specs. Two touches fall outside the feature's own files, both required to keep house gates green: - website/scripts/check-bundle-size.mjs raises the 'all' chunk budget 9200 -> 9300 KB. That chunk is the eager i18n catalogs; this app's 37 keys across 13 locales add 105 KB and the entry's own comment scopes the ceiling to catching a new library, not translated strings. The app's page is behind a lazy import() and lands in its own chunk. - website/src/i18n/locales/hi.json restores one Papyrus string to the informal register hi.md §4 requires. It regressed to the formal आप in d805995 (kirodotdev#5529), which took main to 120 against its own ceiling of 119, so hiStyle fails on main independently of this branch. Fixing the word puts it back at 119 rather than loosening the ratchet. Closes kirodotdev#2516
Summary
Copy-only follow-up to the #699 keeper-half migration (#5242), which moved
window.confirm()call sites into the themed confirm dialog but deliberately froze all user-facing strings. Two copy defects remained, both flagged by the UX review on #5242. Closes #5243.pages.artifactDeployPage.destroy_confirm/recall_confirm/remove_profile_confirm,apps.papyrus.workspace.co_author_conflict_discard_confirm) now open with the consequence and the resource name; the title keeps the question.co_author_conflict_discard/_button/_title) are unified on "changes", with each catalog's term aligned to that catalog's existingdiscard_unsaved_changesvalue. The trigger keeps its "and reload" consequence wording.Values only — no key renames, no component or behavior change. Swept across en + all 11 translated catalogs (locale quote pairs and French U+202F narrow no-break spaces preserved; the
{{file}}operand stays quoted perdestructiveConfirm.test.tsQUOTED_OPERAND_CONFIRM_KEYS);en-XAregenerated viascripts/gen-pseudolocale.mjs.PapyrusPageCoverageselectors updated to the new copy — the failing selectors correctly reflect the change and double as regression pins.Scope note: ru/ko have a pre-existing terminology drift between
pages.artifactDetailPage.discard_unsaved_changesandcomponents.markdownPanel.discard_unsaved_changes(e.g. ko 변경 내용 vs 변경 사항). That predates this PR; this PR aligns the Papyrus keys to each catalog'scomponents.markdownPanelterm and does not sweep the rest — flagged for a follow-up rather than widening a copy-only diff.Known inherited red:
npx tsc -bcurrently fails on origin/main itself (src/hooks/useDialogFocusTrap.ts(85): TS2552 'imeLatchRef', from the crossed #5473/#5482 IME PRs). That file is untouched here; this PR's diff is locale JSON + one test file + a capture harness.Screenshots
Before — the dialog title's question repeated in caps in the body / "edits" verb:
After — body opens with the consequence + resource name; discard verb unified on "changes":
Captured by
website/scripts/capture-confirm-dialog-copy.mjsdrivingwebsite/capture/confirm-dialog-copy.tsx— the REALuseConfirm()dialog with the REAL i18n keys and interpolation (title/body/confirmLabel triples verbatim from the production call sites); each frame is text-asserted before it is written, and the before frames were captured with the catalog edits stashed.Testing
I18N_BASE_REF=origin/main npm run i18n:check— exit 0, all 10 gates ok (incl. diff-scoped changed-values QA and pseudolocale).npx vitest run src/i18n/ src/test/PapyrusPageCoverage.test.tsx— 680 passed (all i18n gates: catalogParity, changedValueQa, destructiveConfirm operand quoting, deadKeys, duplicateKeys, contextSidecar, glossary, pseudolocale).