githubnext/chopinPublic

Let's compose rich plans together

Resolved decisions in prose: margin marker, hover preview, pinned popover #234

MergedMaggieAppletonmerged 4 commits intomainfromrefs/pull/234/head
  1. MA

    ## Summary

    PR 3 of the decision card redesign. **Stacked on #232 (presence) โ†’ #231 (restyle)**; base is `design/decision-card-presence`.

    A resolved decision is now only a margin marker beside the prose it produced.

    - A 20px success disc with the decision icon sits in the left gutter, centred on the first line of the first anchored block. It is a focusable button named `Decision: <answer>`. - Hovering the marker or the anchored prose washes the passage (success tint) and shows a preview in the `.plan-comment-preview` surface and motion: question, chosen answer with a check, decider (18px Face + handle) and date (clock icon). - Pressing the marker or the prose pins the same popover. It grows by a grid-row fold to list the options not chosen, and the 24px close button fades in. Close, Escape and a press outside all dismiss. Only one pin exists. Clearing takes down only the wash this layer put up. - A card whose every question is answered and linked to prose collapses out of the plan. Pending, deliberately empty and orphaned decisions keep the compact resolved card. The Decisions pane is unchanged. - Coarse pointer: the marker's hit target is 44px, like `.plan-comment-button`. - `ClockIcon` and `MessageForwardIcon` are added to `@chopin/icons` (the latter is unused until "Show in chat" has data).

    ## Decisions made

    - **No footer actions.** Nothing in production can reopen or discard an *answered* questionnaire (`question:cancel` only applies to open ones). The footer is omitted rather than shown inert. "Show in chat" and "with ..." are omitted as specified (no data). - **Pin holds until dismissed, not 5 s.** `marks.pin` lapses after 5 s, which is wrong for a popover. The layer holds the wash itself via `highlight` for as long as it is shown, and `clear`s only what it painted. `reveal` is not used: the marker already sits at the prose, so there is no scroll to do and no walk between several blocks. - **Success wash.** Decision prose washes in `--color-success-wash` through a second highlight name (`plan-decision`); comments keep the brand wash. `marks.ts` now tracks the wash per owner. This also changes the Decisions-pane hover wash from brand to success, which matches the target design. - **Popover placement.** It opens under the first anchored block, aligned to its start (as in the jig), flipping above when it would leave the viewport. The comment preview's "right of the marker" would cover the prose being read. - **Narrow gutter fallback.** When the gutter is too narrow for a 20px disc (under 24px; phones, split panes), the marker becomes a slim 4px bar the height of the first line, in the gutter. A disc overlapping the first letters was worse. Tapping the prose opens the same popover. On coarse pointers the bar's button is 44px wide. - **Several questions in one questionnaire.** One marker per linked question. The card collapses only if *all* questions are linked; otherwise the compact card stays so nothing vanishes. - **Unchosen options.** Derived from the joined answer string by whole-label matching (labels may contain ", "). Custom or unmatchable answers (renamed labels, leftovers, duplicates) show no "also considered" list rather than a wrong one. - **Keyboard.** The marker shows the preview on focus. Pressing Enter/Space pins and moves focus into the popover (it is not next in tab order, after the other markers). Closing returns focus to the marker without re-triggering the preview. - **Hover preview is a tooltip, pin is a dialog** (`role` changes with state); folded content and close button are `inert` until pinned. - **A collapsed card leaves a hidden placeholder** (`hidden`, zero margin) so the decorator node still exists; it can no longer be selected and deleted from the plan view (still possible in Decisions context by removing the source). Noted as a trade-off. - Hover on prose uses document-level pointer tracking (as comments do); a click with a non-collapsed selection or a drag never pins. - `@chopin/icons` icons carry a native `<title>`, so the marker is excluded from the design-system icon tooltip (like `.plan-comment-button`); the preview is the tooltip.

    ## Before / after (2x)

    | | Before | After | | --- | --- | --- | | Resolved decision in the plan | ![before](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/before-rest.png?raw=true) | ![after](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/after-rest.png?raw=true) | | Hover preview | n/a | ![hover](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/after-hover.png?raw=true) | | Pinned popover | n/a | ![pinned](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/after-pinned.png?raw=true) | | Narrow (390px), prose tapped | ![before narrow](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/before-narrow.png?raw=true) | ![after narrow](https://github.com/githubnext/chopin/blob/design-pr-assets/pr-234/after-narrow.png?raw=true) |

    ## Checks

    - `bun run fix`, `bun test` (1765 pass, 0 fail), `bun run types` (exit 0), `bun run ci` all green. - Unit: `packages/editor/src/resolved.test.ts` (unchosen derivation, hover/pin state machine, marker and popover placement, card-collapse rule). - E2E (Chromium, own Postgres containers): new `e2e/decision-marker.e2e.ts` (10 tests: gutter geometry, hover preview and wash, prose hover, pin / close / Escape / outside press and wash teardown, one pin at a time, keyboard, scroll, narrow, 44px touch target, compact-card fallback) plus `sidecar`, `responsive-decisions`, `responsive-comments`, `comment-motion`, `focus`, `interface-quality`: 69 passed, 0 failed. - Design-contract exceptions for the new measured-geometry style and presence class are added, and the `line.tsx` / `questionnaire.tsx` hashes renewed.

    ## Not done

    - No "Show in chat" or "with ..." (no data); no footer actions (none exist for answered decisions). - The popover flip is based on its measured height; growing from preview to pinned near the viewport bottom can flip it once.

    ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code)

  2. MA
    Author

    Independent review (08bb6a1f pushed).

    **Fixed** - Escape: a hover-only preview called preventDefault on Escape, which also blocks the child-document Escape handler (it checks defaultPrevented). Now only a pinned popover owns the key. Design-contract hash renewed. - Added e2e: marker follows an edit above its prose (11 tests in decision-marker now).

    **Checked, no change needed** - Highlights: the wash is per owner in marks.ts. Decision wash sits in the `wanted` map, not the single pin, so a comment pin or hover neither clears it nor is cleared by it; store.clear() only touches the questions owner. Effect cleanup clears on unmount, key change and prune. - Decisions-pane wash now success: consistent with the jig (decision prose is success-tinted). Side effect: hovering an unanswered question in the pane also washes success. Low; left. - Geometry, narrow fallback, 44px target, a11y roles and focus return all behave as described.

    **Notes (not changed)** - Arrow-down from the paragraph above a decision card selects the decorator, and typing then inserts at the document start. This predates the PR (reproduced with the visible compact card), but with the card collapsed the selection is invisible, so it is easier to hit. Needs a Lexical-level fix. - Escape while editing makes Lexical blur the editor; the layer then returns focus to the marker. Acceptable.

    **Checks**: bun run fix, bun test (1767 pass), types exit 0, ci green. E2E chromium decision-marker 11/11; fixtures project sidecar, responsive-decisions, responsive-comments, comment-motion 48 passed, responsive-content 10 passed.

  3. MA
    Author

    **Fix for the arrow-down / typing bug** (pushed, see the latest commit)

    **Cause.** Arrowing onto a decorator block gives Lexical a node selection. Its `beforeinput` handler returns early for anything that is not a range selection and does not cancel the event, so Chromium inserts the text into the root at its start, and since the editor selection never moves each character lands in front of the last ("abc" becomes "cba" at the top of the document). Pre-existing, but with the collapsed hidden placeholder the selection was invisible.

    **Fix** (`packages/editor/src/widgets/decorator-selection.tsx`): - `BEFORE_INPUT_COMMAND` cancels insertion while a node is selected. This covers any decorator, including the visible card, which stays selectable and is still deleted by Backspace/Delete. - For a collapsed (hidden) decision, ArrowUp/Down/Left/Right step over it to the neighbouring text block as if it were not there. Vertical moves are left to the browser (column kept), horizontal ones place the caret explicitly.

    **Tests.** Unit: `decorator-selection.test.ts` (skip logic and headless caret placement). E2E in `decision-marker.e2e.ts`: down, up, left/right over a collapsed card, and typing with a visible card selected (all reproduced the bug before the fix). decision-marker 15/15 (chromium), plus editing, focus and sidecar 59/59 together. No design-contract hashes changed.

    **Not addressed.** With a visible card selected, typed text is ignored rather than creating a paragraph. Backspace in the paragraph right after a hidden card still selects the invisible placeholder first (Lexical's behaviour); a vertical move onto a visible card that follows a hidden one still selects the hidden one.

    ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code)

  4. MA
    Author

    **Follow-up** (pushed): the open items from the previous comment are closed.

    - Backspace at the start of the block after a collapsed decision, and Delete at the end of the block before it, now join the two paragraphs as if the placeholder were not there (caret at the join). The placeholder stays and is never selected or deleted by these keys. Non-paragraph neighbours just get the caret. - A node selection that lands on a hidden placeholder is handed on to the next visible block in the direction of travel, so ArrowDown/Up onto a visible card that follows a hidden one selects the visible card.

    Each was reproduced in e2e first (three new tests in `decision-marker.e2e.ts`); unit tests cover the join and release logic. decision-marker 18/18 (chromium); with editing, focus and sidecar 62/62. fix, types, bun test (1775 pass), ci all green.

    ๐Ÿค– Generated with [Claude Code](https://claude.com/claude-code)

  5. MaggieAppletonadded-to-stacklast month on GitHub

  6. MaggieAppletonhead-ref-force-pushed 947f77de1451last month on GitHub

  7. MaggieAppletonbase-ref-force-pushedlast month on GitHub

  8. MaggieAppletonhead-ref-force-pushed 44d0676cea16last month on GitHub

  9. MaggieAppletonautomatic-base-change-succeededlast month on GitHub

  10. MaggieAppletonhead-ref-force-pushed 3165f123807elast month on GitHub

  11. MaggieAppletonhead-ref-force-pushed 6cba2d86fef8last month on GitHub

  12. MaggieAppletonmerged 04e75bff591elast month on GitHub

  13. MaggieAppletonclosedlast month on GitHub

  14. MaggieAppletonhead-ref-deletedlast month on GitHub