Mobile web UI [3/7]: Stop the list pages scrolling sideways on a phone - #295
Mobile web UI [3/7]: Stop the list pages scrolling sideways on a phone#295alex-clickhouse wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Introduces a responsive shared list-page header to prevent horizontal scrolling on mobile.
Changes:
- Adds responsive wrapping and horizontally scrollable filters.
- Migrates Tasks, Plans, and Notifications headers.
- Adds accessible names for icon-only task actions.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
web/src/components/ui/PageHeader.tsx |
Adds the shared responsive header. |
web/src/pages/TasksPage.tsx |
Migrates task controls to PageHeader. |
web/src/pages/PlansPage.tsx |
Migrates plan filters to PageHeader. |
web/src/pages/NotificationsPage.tsx |
Migrates filters and actions with group separation. |
Suppressed comments (1)
web/src/components/ui/PageHeader.tsx:38
w-fullis only the header's content width (the viewport minus 32px of padding). Moving that fixed width left by 16px makes the strip reach the left edge but leaves 32px before the right edge, so it does not provide the documented edge bleed/half-pill affordance. Include both negative margins in the strip width.
<div className="order-2 md:order-none w-full md:w-auto min-w-0
-mx-4 px-4 md:mx-0 md:px-0
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| <div className="border-b border-border-subtle bg-bg shrink-0 px-4 md:px-6 py-2.5 md:py-3 | ||
| flex flex-wrap md:flex-nowrap items-center gap-x-4 gap-y-2"> |
There was a problem hiding this comment.
Fixed, both halves. The single row now returns at lg rather than md, and the filter strip keeps overflow-x-auto at every width instead of flipping to md:overflow-visible. Worth noting the breakpoint alone is not enough: by my arithmetic the Notifications header (nine pills + two actions + title) needs roughly 1150px of viewport once the 56px rail and the padding are accounted for, so it does not fit at lg either. Retaining the scroller is what makes that degrade to an internal scroll instead of spilling into the controls after it.
There was a problem hiding this comment.
Also picked up the suppressed comment on line 38 — you were right that w-full plus -mx-4 only shifts the strip left and leaves it 32px short of the right edge. It is now w-[calc(100%+2rem)], so it spans exactly the viewport and the half-pill affordance works at both edges.
| {actions && ( | ||
| <div className="flex items-center gap-2 shrink-0 order-1 md:order-none md:ml-auto"> |
There was a problem hiding this comment.
Fixed. No flex order anywhere in the header now. Below lg the actions render inside the title row and the desktop copy is dropped from the DOM; from lg up it is the reverse. Only one copy is ever in the tree, so tab order follows what is on screen at both sizes.
| return ( | ||
| <div className="border-b border-border-subtle bg-bg shrink-0 px-4 md:px-6 py-2.5 md:py-3 | ||
| flex flex-wrap md:flex-nowrap items-center gap-x-4 gap-y-2"> | ||
| <div className="flex items-center gap-2 min-w-0 flex-1 md:flex-none order-0"> |
There was a problem hiding this comment.
Fixed. The title wrapper takes lg:gap-4 lg:mr-2 when an icon is present, restoring the 16px icon-to-title gap and the 24px run-out to the filters that /plans and /notifications had. /tasks has no icon, so it keeps its 16px title-to-filter gap.
Every list page had copy-pasted the same header: one non-wrapping flex row holding an icon, a title, one or two runs of filter pills, a search box and the page's action buttons. Measured at 412px that row reaches ~750px, so the *page itself* scrolled horizontally and every filter past the third was unreachable — not clipped-but-scrollable, genuinely unreachable. /notifications 708px overflow "Answered" unreachable /tasks 696px overflow "Done" unreachable /plans 110px overflow Extract it as a PageHeader and migrate the three worst pages. All three now measure 0px of page overflow. Below `lg` the row breaks into title + actions / filters / search, and the filter strip becomes a horizontal scroller bled to both screen edges, so a half-visible pill reads as "swipe for more" rather than as a broken layout. The single row comes back at `lg` rather than `md` because the desktop shell also spends 56px on the nav rail: a 768px viewport leaves ~664px of content, and the widest header (Notifications — nine pills plus two actions) does not fit in it. It does not fit in `lg` either, which is why the strip keeps `overflow-x-auto` at every width instead of reverting to `overflow-visible`. A header that outgrows its container scrolls inside itself; before, it spilled over the controls after it. Three details worth the extra lines: - nothing is reordered with CSS. Below `lg` the actions render in the title row and the desktop copy is dropped from the DOM, and vice versa, so tab order follows what is on screen at both sizes. A flex `order` swap moves boxes but not the tab sequence, which would have left the keyboard stepping through every filter and the search box before reaching the buttons sitting beside the title. - the bled strip is `calc(100% + 2rem)` wide, not `w-full`. The negative margins are 2rem in total, so a full-width strip would only have been shifted left — reaching the left edge but stopping 32px short of the right, which is the edge the half-pill affordance is for. - with an icon, desktop keeps the 16px icon-to-title gap and the 24px run-out to the filters that /plans and /notifications already had; the shared wrapper would otherwise have tightened them to 8px and 16px. /tasks has no icon and keeps its 16px title-to-filter gap either way. The buttons whose labels collapse to icons at `sm` now carry title and aria-label, so they do not become unnamed icon buttons. /notifications gains a divider between its status and type pills: `ml-1` had been enough to separate the two groups only while they sat on a roomy row, and read as one undifferentiated run once they shared a scroller. /memory and /cron use the same header shape and are not migrated here — they need their multi-pane bodies stacked in the same pass. Refs #271
2f22d1c to
c3bef92
Compare
Third step on #271, stacked on #294 (which is stacked on #293). Review those first — this diff only makes sense on top of them.
The problem
Every list page had copy-pasted the same header: one non-wrapping flex row holding an icon, a title, one or two runs of filter pills, a search box and the page's action buttons.
Measured at 412px that row reaches ~750px. The consequence is worse than it sounds — the page itself scrolled horizontally, so filters past the third were not clipped-but-scrollable, they were unreachable:
/notifications/tasks/plans/tasksand/notificationsare two of the four permanent bottom-bar destinations from #294, i.e. the pages most likely to be opened from a phone.The change
Extract the pattern as
ui/PageHeaderand migrate the three worst pages. All three now measure 0px of page overflow.Desktop keeps the original single row. Below
mdit breaks into title + actions / filters / search, and the filter strip becomes a horizontal scroller bled to the screen edge — a half-visible pill reads as "swipe for more" rather than as a broken layout.Notes for review
order-1. I had them earlier in the DOM first, and it silently reordered the desktop header: anml-autoon a middle flex item pushes everything after it right too, so the action buttons landed left of the filters. Worth knowing if this component grows more slots.sm(New Task,Statuses) now carrytitle+aria-label. Hiding the only text in a button otherwise leaves it with no accessible name./notificationsgains a divider between its status and type pill groups. The oldml-1separated them only while they sat on a roomy row; once both groups shared one scroller they read as a single undifferentiated run of nine pills./memoryand/cronuse the same header shape but are not migrated here. Both also need their multi-pane bodies stacked, and doing the header without the body would leave them half-converted. They come as one pass.Verification
npm run buildclean;eslintclean on the touched files (one pre-existingno-explicit-anyinNotificationsPage.tsx:377, outside this change)main: header order, spacing and every control identicalStill to do for #271
/cron+/memorypane stacking,/diagnosticstables, touch-target pass, and the detail pages (/tasks/:id,/plans/:id,/skills/:id,/mcp/:serverName) which I have not surveyed yet.Separately, and unrelated to layout:
/notificationsrenders markdown literally —**PR:**shows as raw asterisks in the body. Broken on desktop too; I did not want to smuggle a content fix into a layout PR.