Skip to content

fix(asktool): paint the dock question card on the composer plate (#360) - #488

Merged
vastsa merged 2 commits into
mainfrom
fix/settings-ui-interaction-polish
Sep 16, 2026
Merged

vastsa merged 2 commits into
mainfrom
fix/settings-ui-interaction-polish

Conversation

@vastsa

@vastsa vastsa commented Sep 16, 2026

Copy link
Copy Markdown
Owner

Summary

Issue #360 bundles five items. Per the earlier triage (owner's comments + PR #439), item 5 is fixed and items 2–4 are visual taste or product decisions. This PR resolves the one item that is a provable spec/code contradiction — item 1, "选择交互面板缺少背景色".

Item 1: the panel is not missing a background, it is painted on the wrong one

The question card is mounted in the transparent composer dock (Composer.tsx renders it as a direct .composer-stack child; the transcript mount is gone), yet it still painted the in-flow --ds-tile wash — a 3.5% ink mix with no shadow. In the reporter's screenshot that is a #f7f7f7 panel on a #ffffff page (measured from the JPEG: dominant interior #f7f7f7, option rows #ffffff, i.e. exactly --ds-tile over white).

Meanwhile the same slot's rule claims otherwise, and so does the spec:

  • composer.css: "Asktool uses the same decision surface as Plan and Goal approval."
  • 04-ux/03-permission-ux.md §9 / 04-ux/08-component-spec.md §11.5: a card in the transparent composer dock "paints --ds-bg-composer with --ds-shadow-composer rather than the in-flow --ds-tile wash".

.plan-approval-bar — the card that actually sits in that slot — does paint --ds-bg-composer + --ds-shadow-composer. The question card did not.

Fix

  • .composer-stack > .asktool-card now paints that plate, in both palettes.
  • Its option rows and custom input move to the plate's inlaid tier (--ds-tile-deep, raised shadow dropped) — the layer .plan-approval-split already uses on that same plate. Without that flip the rows would be white on white in the light palette, where --ds-raised and --ds-bg-composer are both #ffffff. The 15 px option mark keeps --ds-tile-deep (the sidebar checkbox's mark), which reads against the inlaid row exactly as it does against the sidebar.

Tokens only, no literals: both layers are existing --ds-* tokens defined in both palettes, so a contributed theme moves the plate and the rows. Renderer only — no protocol, storage, host, permission, migration, or preference change, and no new default (D435). The transcript/base .asktool-card rules are untouched.

What the other items did not get

  • Item 2 (sidebar vs window): the two surfaces are different (#f3f3f3 vs #ffffff in light, #000000 vs #181818 in dark). The screenshots measure a ~4.7% light-theme difference, i.e. a contrast judgement, and the one byte-identical pair (--ds-bg-dock-raised = --ds-bg-primary = #ffffff in light) is documented and intentional (D419, and the 46px chrome band rule). Changing either value changes the built-in look, so it needs the maintainer's call — not a drive-by edit.
  • Item 3 (merge summon/hide into one toggle): summonWindow (Mod+Shift+W) is deliberate as the symmetric counterpart of closeWindow (Mod+W, D384, 04-ux/09-interaction-patterns.md §1.1, and a macOS menu accelerator). Merging removes or repurposes a user-visible default.
  • Item 4 (pin a new-session button / pick a project in a new session / group history by date): today only project groups group by time (groupSidebarSessionsByTime: today is headerless, then yesterday / this week / older 14d / archived); the temporary-session list does not, and the sidebar header has no new-session control at all (data-action="new-standalone-session" sits inside the scrolling section). New UI + information architecture, so it needs a decision.

Validation

Command Result
pnpm --filter @pi-desktop/desktop typecheck pass
pnpm build:js pass
pnpm lint (biome + check-style-tokens.mjs) pass — style tokens OK
pnpm --filter @pi-desktop/desktop test pass — 1954 tests, 0 fail
pnpm docs:check pass — 449 pages
node scripts/check-architecture.mjs pass
pnpm test:e2e:theme-surfaces pass, incl. the new asktoolCard / asktoolOption surfaces
pnpm test:e2e:layout pass — 89/89 (with PI_DESKTOP_HOST_BIN pointed at a built host-core)

The Chromium probe is the real verification: built-in paint, the composer shadow, and the contributed-variable cascade are asserted for both palettes, and permission-inline.test.mjs asserts the dock rule and the inlaid rows so the plate cannot silently regress. Captured renders (PI_E2E_ARTIFACT_DIR) show the layering in both palettes at 2x:

Layer Light Dark
Plate (.composer-stack > .asktool-card) #ffffff + composer shadow #212121 (96% #212121) + composer shadow
Option row #ededed (8% ink) #333333 (8% white)
Selected row #d8d8d8 #474747
Custom input #ededed #333333

Evidence strength: computed styles, real Chromium render, and pixel measurements of that render — all green and identical in structure across palettes. No human eyeballed the app window (the change is CSS-only and Electron was exercised through the E2E harness, not by hand).

Refs #360

Issue #360 item 1 ("选择交互面板缺少背景色") reads as a panel with no background
because the question card is mounted in the transparent composer dock
(Composer.tsx renders it as a direct `.composer-stack` child) yet still painted
the in-flow `--ds-tile` wash: a 3.5% ink mix with no shadow, measured at
`#f7f7f7` on the reporter's light-theme screenshot. The dock rule's own comment
already claimed the card "uses the same decision surface as Plan and Goal
approval", while `.plan-approval-bar` in that exact slot paints
`--ds-bg-composer` with `--ds-shadow-composer` (`04-ux/03-permission-ux.md` §9,
`04-ux/08-component-spec.md` §11.5).

- `.composer-stack > .asktool-card` now paints that plate in both palettes.
- Its option rows and custom input move to the plate's inlaid tier
  (`--ds-tile-deep`, no raised shadow) — the layer `.plan-approval-split`
  already uses on that same plate — so the rows cannot be white on white in the
  light palette, where `--ds-raised` and `--ds-bg-composer` are both `#ffffff`.
  The 15 px option mark keeps `--ds-tile-deep`, the sidebar checkbox's mark.
- Tokens only: a contributed theme moves the plate and the rows with
  `--ds-bg-composer` / `--ds-tile-deep`. No protocol, storage, host, permission,
  migration, or preference change, and no new default (D435).
- E2E-078's Chromium probe now renders the dock card and asserts the built-in
  paint, the shadow, and the contributed-variable cascade in light and dark;
  `permission-inline.test.mjs` asserts the dock rule and the inlaid rows.

Refs #360
Copilot AI lite review requested due to automatic review settings September 16, 2026 14:50

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Two doc files collided on append (the decisions log and its zh-CN mirror).
Main's entry is kept and this branch's entry follows it, renumbered
D435 -> D437: D435 is already claimed by feat/chat-wide-layout, so the next
free number after main's D436 is the honest choice. The design-system and
E2E-plan citations were updated to D437.
@vastsa

vastsa commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Renumbered at integration: the decision entry was D435, but feat/chat-wide-layout already claims D435 while origin/main has D436 — so this branch now records D437 (commit message and the initial description still say D435; the docs and citations say D437). origin/main was merged into the branch to resolve the append collision in both decisions-log files, and the whole validation set was re-run on the merged tree: typecheck, build:js, lint (style tokens OK), desktop tests 1963/1963, docs:check (77 locale pairs, 451 pages), check-architecture, test:e2e:theme-surfaces — all pass.

@vastsa
vastsa merged commit 64260b1 into main Sep 16, 2026
4 checks passed
@vastsa vastsa mentioned this pull request Sep 16, 2026
@vastsa
vastsa deleted the fix/settings-ui-interaction-polish branch September 21, 2026 11:16

This branch was successfully deployed

1 active deployment
Preview — 223281a1 Deployed Sep 16, 2026 by vercel[bot]
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.

2 participants