githubnext/chopinPublic

Let's compose rich plans together

Quiet accepted-comment row in the document #357

MergedMaggieAppletonmerged 3 commits intomainfromrefs/pull/357/head
  1. MA

    ## Why Accepted comments appeared as large inset cards after the related passage. The provenance came first, and the accessible name called them “Decision” rather than “Accepted comment.”

    ## What changed - Render each accepted comment as a quiet disclosure row with its outcome and short quote visible. - Opening the row reveals the full quote and notes. Provenance moves into the opened body on phones. - Use the shared quote style and disclosure motion while leaving the authoritative comment record and server validation intact. - Renew the reviewed editor design hash after checking the integrated source.

    ## Verification Focused decision tests, `bun run types`, `bun run ci`, and `bun run build` pass on the restacked branch. Required GitHub checks will run on the final published head.

    ## Visuals Before:

    ![Accepted comment before](https://github.com/user-attachments/assets/6c32a421-dbf3-489d-8868-3d7d7119e14b)

    After, closed and open:

    ![Accepted comment closed](https://github.com/user-attachments/assets/9d22548e-9fb4-4d66-8f57-6b0205baa849)

    ![Accepted comment open](https://github.com/user-attachments/assets/909c04df-9bd3-47f2-a536-38d605980e4a)

  2. handlementionedlast month on GitHub

  3. handlesubscribedlast month on GitHub

  4. MA
    Author

    **Review:** Changes needed

    Checked in a real browser (desktop 1440x900, phone 390x844), keyboard and accessibility tree. CI is green. The row itself is a clear improvement over the inset card. The items below are what stop it being right.

    **1. What the row says at rest (design, main point).** The summary is "Accepted · <quote>", which only says where. It never says what was decided. #302 completed cards show the outcome at rest, and this row hides it behind a click. The quote also repeats as the first thing in the opened body, so opening adds nothing for the first line. In the opened state the thread still reads "@dev question, @octocat answer". Suggestion: lead the line with the last note, "Accepted · <last note text>", and keep the quote only in the opened body (or as the secondary part when the note is short). Collapsing is right, because accepted comments are history. Collapsing the outcome with it is not.

    **2. Touch target.** On the phone the row button is 334x20px (`decision.tsx` button, `p-0`). That breaks the #299 ergonomics direction (44px). Give the button `min-h-9`/vertical padding on coarse pointers, or make the whole row padded.

    **3. Instant open, no chevron motion.** Motion is feasible without heavy plumbing. `InlineQuestionnaire` already reads `widgets$` via `useCellValue` and gets `motionImmediately` from it. Add `disclosureMotion?: MotionDisclosureContract` to `WidgetOptions` (`widget-options.ts`), pass `motionContract("collapse")` from `room-workspace.tsx` next to `QUESTION_MOTION` (plan-editor already forwards the options), then in `DecisionCard` use `MotionDisclosure` (as `decisions.tsx` does for history, which also gives `inert`/`aria-hidden` while closing) and `MotionDisclosureIcon` for the chevron. That is about three small files. The chevron currently snaps with `rotate-90` and no transition, unlike the other disclosures in the app. If you would rather ship this slice without it, say so in a follow-up issue.

    **4. `my-0!` overrides (`decision.tsx`, 4 places).** They are needed only because `.plan-content blockquote/p/ul/li` (specificity 0,1,1) beat a utility. A cleaner fix is one rule in `packages/editor/src/styles.css`, e.g. `.plan-content [data-plan-comment-card] :is(p, ul, li, blockquote) { margin-block: 0 }`, or `:where()` on the prose margin rules. Then drop the `!` utilities and the repeated class noise. Note the blockquote also gets the prose `padding-inline-start`/3px border rules, which you override with `border-l-2 pl-3`, so the border width differs from other quotes in the document (3px vs 2px). Prefer reusing the prose blockquote look.

    **5. Alignment and hierarchy in the opened body.** Quote text starts at x=868 and the note list at x=872 (the list has `p-0` but the quote has its border plus `pl-3`). They should share one left edge. The @handle label is `text-xs` brand ink above a `text-sm` paragraph, which is fine, but the note text is full primary ink while the quote is secondary, so the notes look louder than the quote they refer to. OK, but consider tightening.

    **6. Smaller points.** - The right-hand "@octocat · Oct 8, 12:00 AM" is `hidden` below `sm` and moves into the opened body as "Accepted by @octocat". Good. But `sm` is 640px: on a narrow desktop pane the quote truncates early. Consider hiding the stamp when the pane is narrow rather than on viewport width, or accept it. - Truncation: "Accepted · We want to release the new editor t..." is fine. With a short quote there is empty space between the quote and the stamp, which reads as intended. - Row alignment: the icon, label and card column line up with the questionnaire card edge (x=827). Good.

    **Keyboard / screen reader (verified).** Enter and Space both toggle, `aria-expanded` flips, focus stays on the button. `aria-controls` is only set while open, which is correct. The accessibility tree reads "article Accepted comment" containing button "Accepted · <quote> @octocat · Oct 8, ..." [expanded], then blockquote, list. Good. Minor: the button's accessible name includes the stamp text. That is fine, but `aria-label` is unnecessary. The focus ring does show (global), but with `p-0` it sits flush against the icon and chevron and touches the text. Adding vertical or horizontal padding (which also fixes point 2) would give it room.

    **Tests.** The updated unit test only asserts the collapsed markup. Add a test for the open state (notes and quote order) if the component can be tested by rendering with the state forced, or cover it with an e2e check that clicking the row reveals the notes. CI e2e is green, which suggests existing flows are unaffected, but nothing asserts the new behaviour.

    **PR body.** The three `./...png` relative image links were broken (those files are not in the repo). I removed them and kept the three working `user-attachments` images under Screenshots (all return 200 image/png).

    Screenshots from this review: `.design-orchestrator/audit-2026-10-07/shots/review-357/` (`d-rest-full.png` at rest, `d-open.png`, `m-rest.png`, `m-open.png`).

  5. octocatmentionedlast month on GitHub

  6. octocatsubscribedlast month on GitHub

  7. handlementionedlast month on GitHub

  8. handlesubscribedlast month on GitHub

  9. devmentionedlast month on GitHub

  10. devsubscribedlast month on GitHub

  11. MaggieAppletonhead-ref-force-pushed 8a954b948abflast month on GitHub

  12. MA
    Author

    Addressed in 8a954b94:

    1. Collapsed row now leads with the outcome ("Accepted · <last note>"); the quote appears only in the opened body. 2. Row button has padding (`px-1 py-1.5`, rounded, so the focus ring has room) and `min-height: 44px` under `data-plan-coarse-pointer`. 3. Motion: `disclosureMotion` added to `WidgetOptions`, forwarded by `PlanEditor`, supplied as `motionContract("collapse")` in `room-workspace.tsx`. `DecisionCard` uses `MotionDisclosure` and `MotionDisclosureIcon` (inert/aria-hidden while closing). 4. The four `my-0!` are gone; one scoped rule in `styles.css` zeroes margins and the quote reuses the prose 3px rule. 5. Quote and notes now share one left edge (measured on desktop and phone). 6. Added an open-state unit test (quote before notes).

    Checks: `bun test` for the widget passes, `bun run types` passes. `bun run ci` fails only the design-contract check: `packages/editor/src/plan-editor.tsx` (exception in `scripts/design-contract/exceptions/dynamic-editor.json`, case `class > PlanEditor > <div>`). The new sha256 is `21a950f565d853e151c761b869d70aa082787ee3929d7cca36d51d8c2050f468`. The change only forwards one prop and adds no class logic, so the reviewed `className` case is unchanged.

  13. MaggieAppletonreferenced this in Show each decision's relationship to the document #367last month on GitHub

  14. MaggieAppletonhead-ref-force-pushed db1d62471b02last month on GitHub

  15. MaggieAppletonauto-rebase-enabledlast month on GitHub

  16. handleunsubscribedlast month on GitHub

  17. MaggieAppletonmerged ef8cf3a2c2c0last month on GitHub

  18. MaggieAppletonclosedlast month on GitHub

  19. MaggieAppletonhead-ref-deletedlast month on GitHub