Skip to content

Phone comments list, Build empty state and a Chopin chip - #495

Merged
MaggieAppleton merged 6 commits into
mainfrom
mobile/comments-and-empty-states
Oct 12, 2026
Merged

MaggieAppleton merged 6 commits into
mainfrom
mobile/comments-and-empty-states

Conversation

@MaggieAppleton

@MaggieAppleton MaggieAppleton commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Follow-up to the phone redesign: gaps 4–8 from the Mobbin comparison (comments list, Build empty state, top bar, Chopin chip). Phone only; desktop is unchanged.

Verification on current main (iPhone 15 Pro and iPhone SE 375×667)

Gap What main does Verdict
4. Comments entry point There is no way to see all comments. The only entry points are each passage's margin chip. The title's ◯1 chip already reaches 44×45px (measured with elementFromPoint). Fixed: the list was missing. The badge was already fine and is unchanged.
5. "1 of N" paging A passage opens one thread with ‹ 2 of 4 › paging. A list exists only for several threads on one line. Fixed by reusing that list mode. Paging is kept.
6. Build empty state "Answer 2 decisions first" is a lone link at the top left of a blank pane. Left to #508 (see Adapted to #508)
7. Top bar Documents uses DocumentsIcon (from #478) and ⋯; both are 44×44 with 18px glyphs. Already fine. Comments joins the right side; sizes are unchanged.
8. Addressing Chopin Only a teal @chopin underline and an unlabelled music-note glyph on the mode switch. With the switch on and no mention, the note glyph is the only signal. Fixed

What changed

  • Top bar comments button (phone-header.tsx): a MessageIcon with the open-thread count, 51×44, named "4 open comments". It is hidden when no threads are open. Tapping it switches to Document and opens the comment sheet as a list. The left and right slots now share a width, so the title stays centred.
  • "All comments" list (comment-layer.tsx, new openCommentList event in threads.ts): this reuses the sheet's existing list mode (N comments heading, ThreadList) for every open thread in reading order. With quotes, each item quotes its passage. It opens at the medium detent. Tapping an item expands its thread with the existing ‹ All N comments back row, and back returns focus to that item. Closing returns focus to the top bar button.
  • Pager count button (comment-sheet.tsx): inside a thread, the 2 of 4 count now opens the same list (44px target, named "2 of 4, show all comments"). The ‹ › steps are unchanged, so Dock phone selection actions and give the comment sheet detents #474's detent behaviour and the existing pager tests are not affected.
  • Build empty state (removed, see Adapted to Make Build plan explicit and show build status in one capsule #508) (build-view.tsx): a phone's blocked Build tab uses the shared EmptyState (compact): wrench icon, "Build unlocks after decisions", "2 decisions are still open." and a 44px primary Go to Decisions button. It is centred between the bars. A comments blocker shows "Go to Document"; when both blockers apply, both buttons appear. Other empty states are already consistent: Decisions uses EmptyState with a title, description and action, and Chat uses EmptyState without an action, which is left as is because an action would have no real job there.
  • Chopin chip (chat.tsx, composer.css): on a phone, while the message addresses Chopin (switch on or an @chopin mention), a ♪ Chopin × chip sits on its own row inside the pill. It reflects effectiveMode and has no state of its own. Tapping it calls the existing toggleMode, which removes the mention as switching to Chat does. The chip is drawn 24px tall with a 44px touch target.

Screenshots

Top bar, before → after (iPhone 15 Pro):
Top bar before and after

Comments: main's paged thread → the new list from the top bar → a thread opened from the list:
Comments before and after

iPhone SE, document and comments list:
SE comments

Build tab, before → after (15 Pro, then SE):
Build before and after

Composer addressing Chopin, before → after (15 Pro, then SE):
Chopin chip before and after

Desktop 1440×900, before and after (pixel diff 0):
Desktop before
Desktop after

Testing

  • New e2e:
    • comment-sheet.e2e.ts: the top bar list, its order and quotes, the medium detent, the item → thread → back flow and focus return; the pager count opening the list; no comments button on desktop.
    • implementation-launcher.e2e.ts: the phone's blocked Build empty state, centred, with a 44px Go to Decisions that switches tab. The desktop link test is unchanged.
    • chat-phone.e2e.ts: the chip's placement and 44px reach, removal from the mention and from the switch, and no overflow.
  • bun run types ✅, bun test packages/editor apps/web ✅ (1411 pass), lint and format on changed files ✅, design record and impeccable ✅.
  • PR CI e2e ✅ (full suite). The first run caught the Build test measuring while the pane was still sliding in; it now waits for the pane to settle. Local bun run e2e timed out waiting for Playwright's web server.
  • CI validation: design-contract exceptions renewed (see below).
  • Checked by hand on a fake-GitHub server with Playwright (iPhone 15 Pro, iPhone SE, desktop).

Review follow-up (second commit)

  • A new CommentIcon (packages/icons/src/comment.tsx) is a bubble holding two text lines. It marks the top bar button and the phone margin chips, so neither reads as Chat. Desktop chips keep MessageIcon, and desktop stays pixel-identical (diff 0).
  • The top bar uses equal minmax(max-content, 1fr) sides around a max-content title, which puts the title on the exact centre (measured 196.5/196.5 on 15 Pro and 187.5/187.5 on SE). Empty slots keep 44px.
  • A thread opened from a list keeps its passage quote under the "All N comments" back row.
  • The "2 of 4" count is a quiet inset pill with a pressed state.
  • The mode switch is hidden while the Chopin chip shows. Removing the mention, by the chip or by Shift+Tab, no longer leaves a leading or doubled space.
  • The comments-blocked copy now reads "Build unlocks once accepted comments are applied" / "N accepted comments are waiting to be applied."
  • The list's available check now uses the same open, not orphaned, not drifted rule as the top bar count. The duplicate .build-blocked-actions .btn rule is gone.
  • The e2e tests now also check: title centring, the quote under the back row, the switch hidden while the chip shows, and the exact draft text after removal.
  • Not done (optional): a last list row for orphaned comments.

Updated screenshots:
Top bar with comment icon
Comments list, thread with quote, pager pill
Chip replaces the switch; removal leaves no leading space

Design-contract review

Reviewed and renewed in the "Renew design-contract review exceptions for phone comments" commit.

  • packages/editor/src/comment-layer.tsx is pinned twice in scripts/design-contract/exceptions/dynamic-editor.json with sourceHash 6776932ad1c0842647fa490b87ad42234f71bcde1e1fe52f4db8827f9a9a9a21. The new hash is b3ba35a387b51602f42db5f1158514b31146e1f33679e17d88f8400ca115db30. The change is safe: it adds an "all" pin state and list content, and no style or class logic changes in the reviewed preview.style, surface.style, hit or presence class owners. The only rendering change is a phone-only choice between two icons.
  • packages/icons/src/comment.tsx (new) needs the same exception that list.tsx has in scripts/design-contract/exceptions/dynamic-packages.json ("Icon wrapper forwards SVG accessibility, sizing, and caller styling props"), with sourceHash c39ae437a205dcaaad06e7f58b7c112c074e10341ecec8e21e7ff27ccb4130e4. It is a plain LineIcon path.

Needs a real device: chip tap feel above the keyboard, and the list sheet's medium-detent drag on iOS Safari.

Adapted to #508

The phone Build empty state ("Build unlocks after decisions") is removed. #508 keeps the Build tab off until a build exists, and Build plan carries the reason it can't run yet. A never-built document therefore can't open a blocked Build tab, and the empty state would become unreachable once #508 lands. Removing it leaves this PR compatible with main and with #508. Removed: the build-view.tsx branch, the .build-blocked* CSS, and the phone e2e test with its seedOpenDecision helper. Gap 6 is now covered by #508's design. Comments, the top bar and the Chopin chip are unchanged.

🤖 Generated with Claude Code

Top bar with comment icon

Comments list, thread with quote, pager pill

Chopin chip

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Review: Changes needed (one small must-fix, the icon; the rest is solid)

I built origin/main (8850) and this branch (8849) on fake GitHub, seeded 4 open threads and 1 open decision, and drove both with headless Playwright on iPhone 15 Pro, iPhone SE and a 1440×900 desktop.

Must-fix

  1. The comments button reuses the Chat tab's glyph (apps/web/src/phone-header.tsx:143). MessageIcon is the Chat tab icon (phone-workspace.tsx:22), and the mode switch's "Chat" state uses it too. On every tab, "◯ 4" at the top sits above "◯ Chat" at the bottom, so it reads as "4 unread chat messages", like the Decisions badge next to it. Notion and Dropbox use a separate comment glyph from their chat/inbox glyph. The margin chips (comment-layer.tsx:1361) also use MessageIcon, so "use the chip's glyph" does not solve this. Fix: add a CommentIcon to packages/icons/src/line.tsx, e.g. a speech bubble holding two short text lines or a quote mark (SF Symbols text.bubble / quote.bubble), and use it in the top bar. Ideally the phone margin chips use it too, so the button visibly matches the chips. If the desktop chip must stay unchanged, change the top bar only.

Verified working

  • Count: the header shows 4 and the list heading says "4 comments". Excluding orphaned and drifted threads is acceptable: they still have their own "lost their passage" entry in the margin, and an orphan has no passage to quote or scroll to. Nice-to-have: a last row in the list such as "1 comment lost its passage ›" that opens the orphans sheet, so the list really is all comments.
  • List → thread → back: focus goes Close → "All 4 comments" back row → the item you came from. Close and Escape return focus to the top bar button. Dragging the sheet down dismisses it and returns focus. The button opens the list from the Chat tab too.
  • Detents (Dock phone selection actions and give the comment sheet detents #474): the list opens at medium (y=329 of 852 on 15 Pro; 284 on SE) with the document visible. Dragging up enlarges it. Each new thread opens at medium again, as Dock phone selection actions and give the comment sheet detents #474 specifies. The pager is hidden under the "All N comments" back row, as Dock phone selection actions and give the comment sheet detents #474 specifies. The existing ‹ › stepping is unchanged.
  • Pager count: a 45×44 button with the name "2 of 4, show all comments". It opens the list with the current thread focused.
  • Build empty state: a big improvement on main's lone "Answer 1 decision first" link at the top left. Centred on both phones. The primary button is 44px and switches to Decisions.
  • Chopin chip: derived from effectiveMode with no state of its own. Tapping it calls toggleMode, which removes @chopin, and focus stays in the draft (DIV Message), so the keyboard stays up. A 24px chip with a 44px reach. Matches ChatGPT's "Think ×".
  • Desktop: before and after screenshots are byte-identical. Every new path is gated on phone or compact.

Nice-to-have

  • apps/web/src/build-view.css: .build-blocked-actions .btn { min-height } is declared twice, once near line 182 and again at the end of the file. Delete one.
  • phone-workspace.css:300: the slot min-width is 88px, but the right slot holds 51 + 44 = 95px. The title is off-centre by about 3px (157 vs 160 on SE; 193 vs 196.5 on Pro). Size the slots from the real content, or use a 1fr auto 1fr grid on .phone-top-bar.
  • A thread opened from the list loses its passage quote: the back row takes the heading, and the quote line does not render. The pager view shows it. Render the quote under the back row so you keep context after leaving the list.
  • The "2 of 4" button looks like plain text. Give it a subtle pill or tint on :active so people can find it.
  • Chip: when on, Chopin shows three times: the chip, the teal @chopin underline and the music-note glyph on the switch. Consider showing the switch's off glyph or nothing while the chip is visible, as ChatGPT's chip replaces the tool icon. Also, removing the mention leaves a leading space (" what is left?"). This comes from the existing toggleMode, not this PR, but the chip makes it more common. Trim it there.
  • Copy: "Build unlocks after comments" / "accepted comments still need to reach the document" is vague. Try "Build unlocks once accepted comments are applied".
  • comment-layer.tsx:911: available for ALL checks !orphaned but not drifted, while the list renders only placed threads. If only drifted threads remain, pinned stays "all" and nothing renders. Use the same predicate as openComments.

@MaggieAppleton

Copy link
Copy Markdown
Collaborator Author

Review: Looks good

I re-checked head 7d5a0f91 in a fresh worktree on fake GitHub, with the same seed as before, on iPhone 15 Pro, iPhone SE and a 1440×900 desktop.

  • Icon: CommentIcon is a rounded-rectangle bubble with two text lines. Next to the Chat tab's round, empty bubble it now clearly reads as comments rather than chat, in the top bar and on the phone margin chips. Desktop chips keep MessageIcon.
  • Centring: the title is centred: 196 of 196.5 on 15 Pro and 160 of 160 on SE. It was about 3px off before.
  • Quote under the back row: a thread opened from the list now shows its passage beneath "‹ All 4 comments".
  • "2 of 4" pill: it now reads as tappable. It is 61×44 and opens the list with the current thread focused.
  • Chopin chip: the mode switch hides while the chip shows. Removing the chip leaves "what is left?" with no leading space, and focus stays in the draft.
  • Build copy and available rule: both updated as asked.
  • Unchanged behaviour: list → thread → back, focus return on Close and Escape, opening from the Chat tab, and the medium detent (y=329 on Pro, 284 on SE) all still work.
  • Desktop: unchanged. The only pixel difference from main was the sidebar's live decision-count badge loading in. That count is unrelated to this PR.
  • CI: e2e and container pass. Validation fails only on the two design-contract items listed in the PR body: the renewed comment-layer.tsx hash, and a new dynamic-packages.json exception for packages/icons/src/comment.tsx. The new exception is a fresh entry rather than a renewed hash, but it is the same standard icon-wrapper exception that list.tsx has. Lint shows warnings only.

Still worth checking on a real device, as the PR body says: how the chip feels to tap above the keyboard, and dragging the list sheet's medium detent in iOS Safari.

MaggieAppleton and others added 6 commits October 12, 2026 12:07
The phone top bar gains a comments button with the open-thread count that
opens the comment sheet as a list of every open thread, in reading order and
quoting each passage. A thread opened from the list leads back to it, and a
thread's "2 of 4" position opens the same list. A phone's blocked Build tab is
now an EmptyState with a "Go to Decisions" action, and the phone composer shows
a removable Chopin chip while a message addresses the Planner.

Why: apps/web/src/room-workspace.tsx | the top bar's open-comment count filters open, not orphaned, not drifted threads so it matches the sheet's list of placed threads
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A CommentIcon (a bubble holding text) marks the top bar's comments button
and the phone's margin chips, so neither reads as Chat. The top bar title
sits on the true centre line, a thread opened from the list keeps its
passage beneath the back row, the pager position reads as a button, the
switch steps aside while the Chopin chip shows, removing the mention leaves
no leading space, and the list's availability matches the top bar's count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- packages/icons/src/comment.tsx: new icon forwards IconProps to LineIcon
  exactly like list.tsx with static SVG paths, so it gets the same reviewed
  props-forwarding exception.
- packages/editor/src/comment-layer.tsx: adds the phone-only list of every
  thread and a choice between CommentIcon and MessageIcon at the same fixed
  sizes; the chip's style object and class composition are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Build capsule (#508) keeps the Build tab off until a build exists and
explains what blocks Build plan in its own reason, so a blocked Build tab
on a phone no longer appears for a document that was never built. Drop the
phone empty state and its test rather than land a screen that is about to
become unreachable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaggieAppleton
MaggieAppleton force-pushed the mobile/comments-and-empty-states branch from 343d512 to f48fcdb Compare October 12, 2026 11:07
@MaggieAppleton
MaggieAppleton merged commit a9d616a into main Oct 12, 2026
3 checks passed
@MaggieAppleton
MaggieAppleton deleted the mobile/comments-and-empty-states branch October 12, 2026 11:25
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.

1 participant