Mobile web UI [4/7]: Collapse the cron and memory side panes into drawers - #296
Mobile web UI [4/7]: Collapse the cron and memory side panes into drawers#296alex-clickhouse wants to merge 1 commit into
Conversation
alex-clickhouse
changed the title
There was a problem hiding this comment.
Pull request overview
Makes Cron and Memory layouts mobile-friendly by moving side panes into drawers and standardizing headers.
Changes:
- Adds reusable pane toggle controls.
- Converts Cron and Memory sidebars into mobile drawers.
- Fixes responsive
PageHeaderwrapping behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
web/src/pages/MemuPage.tsx |
Adds responsive memory drawer and header. |
web/src/pages/CronPage.tsx |
Adds responsive cron drawer and header. |
web/src/components/ui/PaneToggle.tsx |
Introduces the drawer toggle control. |
web/src/components/ui/PageHeader.tsx |
Corrects responsive title and row sizing. |
web/src/components/Cron/CronSidebar.tsx |
Supports drawer-specific sidebar sizing. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Close the drawer when the tab changes or the layout crosses the | ||
| // breakpoint. During render, not in an effect, so the overlay is never | ||
| // painted over the content for a frame first. | ||
| const paneScope = `${isMobile}:${activeTab}`; |
There was a problem hiding this comment.
Fixed. Sidebar takes an onSelect that fires from both the category buttons and the Clear-filter button, and the drawer closes on it. Creating a category happens inside the pane and deliberately does not fire it. Dropped the scope watcher for this — it would also have missed re-tapping the active category, which clears the filter, so the list underneath does change.
| // Shut the drawer once a job is picked, and whenever the layout crosses the | ||
| // breakpoint. Adjusted during render, not in an effect, so the overlay is | ||
| // never painted over the table for a frame first. | ||
| const paneScope = `${isMobile}:${selectedJobId}`; |
There was a problem hiding this comment.
Fixed. CronSidebar takes an onSelect that fires after every plain selection, including "All Jobs" and re-picking the job already selected, and CronPage closes listOpen directly from it. The modified-click and middle-click paths open a new tab and select nothing, so they do not fire it. The remaining render-time adjustment only handles crossing the breakpoint.
Both pages split the viewport into fixed columns. At 412px that left /cron's run table showing two of its six columns, and gave /memory's three panes roughly 130px each. Below `md` the list pane becomes a left drawer, matching the chat session list — same anchor, same toggle, same icons — so "show me the list" is the same gesture in the same corner everywhere. /cron's table goes from two visible columns to five; /memory's facts get the full width. Both headers move to PageHeader in the same pass. Converting the body without the header would have left them the only pages still scrolling sideways from the old header row. PageHeader gains a `leading` slot for the pane toggle, so it sits where the chat header's sidebar toggle sits. Both drawers close the moment a selection is made, driven by the pane's own handler rather than by watching the selected id. Watching the id misses the case where the tap picks what is already picked: on /cron that is "All Jobs", on /memory it is the active category (which clears the filter, so the list underneath does change). Either way the drawer would have stayed parked over the content it was asked to reveal. Crossing the breakpoint still retires the drawer, adjusted during render so the overlay is never painted over the content for a frame. MemuPage's hooks go above its `if (loading) return` — added below it they changed the hook order between renders and blanked the page as soon as loading flipped. Desktop is unchanged on both pages. Refs #271
def0406 to
b91bc35
Compare
Fourth step on #271, stacked on #295 → #294 → #293. Review the stack in order.
The problem
Both pages split the viewport into fixed columns and never reconsider it. At 412px:
/cron— a 220px job list next to the run table left the table showing two of its six columns (Job, Started). Duration, Status, Output and the rest were off-screen./memory— a 208px types/categories rail, the facts list and the detail pane, all sharing 412px: roughly 130px each.The change
Below
mdthe list pane becomes a left drawer, matching the chat session list from #293 — same anchor, same toggle button, same icon pair. "Show me the list" is now the same gesture in the same corner on every page that has a list./cron's table goes from two visible columns to five;/memory's facts get the full width.Both headers move to
PageHeaderin the same pass. Converting the bodies without the headers would have left these two as the only pages still scrolling sideways from the old header row — half-converted is worse than either end state.A latent bug in #295, found here
Worth calling out because it changes a line that PR added.
The
PageHeadertitle wrapper usedflex-1, which isflex: 1 1 0%. A zero flex-basis means the title contributes nothing to flex line-fitting, so the filter row packs onto the same line and squeezes the title to zero width.It looked correct on the three pages in #295 purely by accident: each has visible action buttons whose width pushed the line past its limit, forcing the wrap I wanted.
/memoryhides its counts belowlg, so nothing pushed the line over — and the title rendered at 0px wide, invisible, while the tabs sat where it should have been.Two changes, so the layout no longer depends on a neighbour to overflow the line:
flex-auto— content-width basis, still truncating viamin-w-0basis-full, taking a line of their own unconditionallyVerified across all five migrated pages: page overflow 0px everywhere, and every
<h1>measures a real width (56/55/129/96/182px).Notes for review
MemuPage's hooks had to go above itsif (loading) return. Added below it, they changed the hook order between renders and blanked the whole page the momentloadingflipped to false. React threw "change in the order of Hooks"; the page rendered as a black rectangle.CronSidebarandMemuPage'sSidebartake aninDrawerprop that drops the fixed width and their own right border — the drawer supplies both./cronrun table still scrolls horizontally within its own container at five columns. That is the P2 table work, deliberately not in this PR.Verification
npm run buildclean;eslintclean on all touched files (one pre-existing warning inMemuPage.tsx:595)main: both pages unchanged — inline sidebars, same header order, same controlsStill to do for #271
P2 tables (
/cron,/diagnostics), the touch-target pass, and the detail pages (/tasks/:id,/plans/:id,/skills/:id,/mcp/:serverName) which remain unsurveyed.