Repository navigation
Phone comments list, Build empty state and a Chopin chip - #495
Merged
Merged
Conversation
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
Verified working
Nice-to-have
|
Collaborator
Author
|
Review: Looks good I re-checked head
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
force-pushed
the
mobile/comments-and-empty-states
branch
from
October 12, 2026 10:04
7d5a0f9 to
1d3c0d3
Compare
MaggieAppleton
enabled auto-merge (rebase)
October 12, 2026 10:05
MaggieAppleton
force-pushed
the
mobile/comments-and-empty-states
branch
from
October 12, 2026 10:52
8a76b4d to
343d512
Compare
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
force-pushed
the
mobile/comments-and-empty-states
branch
from
October 12, 2026 11:07
343d512 to
f48fcdb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)
◯1chip already reaches 44×45px (measured withelementFromPoint).‹ 2 of 4 ›paging. A list exists only for several threads on one line.DocumentsIcon(from #478) and ⋯; both are 44×44 with 18px glyphs.@chopinunderline and an unlabelled music-note glyph on the mode switch. With the switch on and no mention, the note glyph is the only signal.What changed
phone-header.tsx): aMessageIconwith 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.comment-layer.tsx, newopenCommentListevent inthreads.ts): this reuses the sheet's existing list mode (N commentsheading,ThreadList) for every open thread in reading order. Withquotes, each item quotes its passage. It opens at the medium detent. Tapping an item expands its thread with the existing‹ All N commentsback row, and back returns focus to that item. Closing returns focus to the top bar button.comment-sheet.tsx): inside a thread, the2 of 4count 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 sharedEmptyState(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 usesEmptyStatewith a title, description and action, and Chat usesEmptyStatewithout an action, which is left as is because an action would have no real job there.chat.tsx,composer.css): on a phone, while the message addresses Chopin (switch on or an@chopinmention), a♪ Chopin ×chip sits on its own row inside the pill. It reflectseffectiveModeand has no state of its own. Tapping it calls the existingtoggleMode, 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):

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

iPhone SE, document and comments list:

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

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

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


Testing
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 ✅.bun run e2etimed out waiting for Playwright's web server.Review follow-up (second commit)
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 keepMessageIcon, and desktop stays pixel-identical (diff 0).minmax(max-content, 1fr)sides around amax-contenttitle, 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.availablecheck now uses the same open, not orphaned, not drifted rule as the top bar count. The duplicate.build-blocked-actions .btnrule is gone.Updated screenshots:



Design-contract review
Reviewed and renewed in the "Renew design-contract review exceptions for phone comments" commit.
packages/editor/src/comment-layer.tsxis pinned twice inscripts/design-contract/exceptions/dynamic-editor.jsonwithsourceHash6776932ad1c0842647fa490b87ad42234f71bcde1e1fe52f4db8827f9a9a9a21. The new hash isb3ba35a387b51602f42db5f1158514b31146e1f33679e17d88f8400ca115db30. The change is safe: it adds an"all"pin state and list content, and no style or class logic changes in the reviewedpreview.style,surface.style,hitor 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 thatlist.tsxhas inscripts/design-contract/exceptions/dynamic-packages.json("Icon wrapper forwards SVG accessibility, sizing, and caller styling props"), withsourceHashc39ae437a205dcaaad06e7f58b7c112c074e10341ecec8e21e7ff27ccb4130e4. It is a plainLineIconpath.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.tsxbranch, the.build-blocked*CSS, and the phone e2e test with itsseedOpenDecisionhelper. Gap 6 is now covered by #508's design. Comments, the top bar and the Chopin chip are unchanged.🤖 Generated with Claude Code