Conversation
Standalone pages had no shared shell, so each one hand-rolled its frame by dropping a PageTopbar into the default padded `.page-container`. The bar carries the sidebar's surface and a bottom border precisely because it is chrome meant to pin to the frame — inside a padded, max-width-capped well it instead floated in the middle of the content area with a gutter on every side. PageLayout gives those pages the same frame TabGroupLayout already gave a tab group: full-bleed shell, top bar flush to the top of the content region, body scrolling beneath it in the reading column. The shell rules now live in three mixins shared by `.sk-page` and `.sk-tabgroup` so there is one definition of what a page frame is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fourteen pages each carried their own frame: some a padded `.page-container`, some a bare fragment, some the `app-detail-page` chrome with a per-page `.main-content:has(...)` escape hatch to undo the frame again. All of them now render through PageLayout, which deletes the per-page shell code and puts every top bar flush against the content region. Loading and not-found early returns take the same shell as the loaded page, so the bar no longer appears, shifts, or vanishes as data resolves. The now-dead `.main-content:has(.ws-detail-page)` rule goes with it — the shared full-bleed rule covers it, and two :has() rules fought each other on small screens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
components/_schedule-picker.scss was never added to main.scss, so the shared cron picker has been rendering unstyled everywhere it is used — raw bordered boxes with the preset label and its cron expression jammed together, and the next-run list running into the description. Adding the one import restores it for Backups' schedule card and the server-detail cron tab as well. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Creating a job went through a plain modal with a preset <select> and a bare cron text field — the schedule was something you had to already know how to write. It now opens a drawer built on the shared SchedulePicker (presets · builder · advanced, with the humanized cadence and the next fire times), plus a preview of the exact crontab line the form will write. The drawer owns its own form state, so the page drops the form object, the reset/open/submit handlers and the /cron/presets fetch that only fed the old dropdown. The list head gains the section title and the "N of M jobs" count, with the filter moved to the right where the mock has it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`.sk-drawer__body` is the scroll region and deliberately carries no padding — some drawers run edge-to-edge — so each body pads its own root. `.cron-form` did not, leaving every label and input flush against the panel edges. Same inset as `.cron-drawer` so the two cron drawers line up, plus a note on the DS Drawer so the next body does not repeat it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The console panes were rounded, bordered cards floating inside a `0 16px 16px` inset — the only page framed that way, which read as stray side padding plus stray corners against an otherwise flush layout. It now renders through PageLayout in `fill` mode, like the File Manager: the panes fill the content region edge-to-edge and keep their internal dividers. Services stays a card grid, so it re-pads itself instead of running to the edges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Git hand-rolled the tab-group frame — `page-container--full-bleed sk-tabgroup` plus its own `__content`/`__inner` wrappers, duplicated across the loading branch and the main return. It renders through the shared layout now, so the frame comes from one place and the loading state matches the loaded page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…owning bodies Every drawer body had to remember to pad itself, and they had drifted into three answers: 18px (filters, telemetry, incidents, monitors), 20/20/32 (domains, cron), and nothing at all — backup detail, restore, cutover and the scheduled-tasks runs list were rendering flush against the panel edge, the same defect just reported on the cron create drawer. Ten drawers, three insets. The body is now padded to the header's own 18px inset, and the seven bodies that genuinely own their internal regions — a form with a pinned footer (Templates, WordPress), a two-pane catalog (both database engine drawers), an editor (widget editor), a filtered library (widget library), a mode switcher (Servers) — declare `flush` instead of relying on the absence of a default. The now-duplicate per-drawer padding is deleted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three detail panels rendered their own `.preview-drawer` — fixed positioning, their own backdrop, their own slide animation, their own close button — so they had no focus trap, no escape-to-close, and header chrome that only resembled the shared drawer's. The Terminal process and service panels and Docker's container logs panel now use ds/Drawer like everything else. Left alone deliberately: the File Manager preview (it also renders inline as a column pane, which a Radix sheet cannot do) and Docker's container inspector (the `dx-*` workspace has its own language by design). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
components/PageSkeleton.jsx already owned `.sk-page` for its loading archetypes, including `gap: $space-4`. The new shell reused the name, so every page built on PageLayout inherited a 16px flex gap between the top bar and the content — invisible on padded pages, but on Terminal/Logs it showed as a band of page background under the bar, which is exactly the "weird space" reported. The shell is `.sk-pageshell` now; the skeleton keeps `.sk-page`. The smoke test gained a vgap assertion (content top must equal the bar's bottom edge), which is what would have caught this in the first place. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes the frontend’s “page shell” and drawer surfaces by introducing a shared PageLayout for standalone routes (matching the existing tab-group shell), making ds/Drawer pad its body by default, and rebuilding Cron Jobs’ create/edit flow around the shared SchedulePicker inside a drawer.
Changes:
- Add
frontend/src/layouts/PageLayout.jsxand refactor many standalone pages to render through it (including loading/not-found branches). - Make
ds/Drawerbody padded by default (withflushopt-out) and migrate remaining hand-rolled slide-overs to the shared drawer. - Rebuild Cron Jobs create/edit UX as a drawer using
SchedulePicker, and wire in the previously-missing schedule picker stylesheet import.
Reviewed changes
Copilot reviewed 37 out of 44 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
VERSION |
Bumps application version to 1.7.76. |
frontend/src/styles/pages/_workspaces.scss |
Removes a :has() padding escape hatch now covered by the shared shell. |
frontend/src/styles/pages/_terminal.scss |
Adjusts Terminal/Logs styling for PageLayout fill full-bleed behavior. |
frontend/src/styles/pages/_telemetry.scss |
Removes per-drawer padding now covered by default drawer-body padding. |
frontend/src/styles/pages/_monitors.scss |
Removes per-drawer padding now covered by default drawer-body padding. |
frontend/src/styles/pages/_domains.scss |
Removes per-drawer padding now covered by default drawer-body padding. |
frontend/src/styles/pages/_cron.scss |
Updates Cron list header styling and adds cron create/edit drawer form styles. |
frontend/src/styles/main.scss |
Imports components/_schedule-picker so SchedulePicker is styled. |
frontend/src/styles/layout/_main-content.scss |
Factors page shell into shared mixins; adds .sk-pageshell alongside .sk-tabgroup. |
frontend/src/styles/components/_design-system.scss |
Makes .sk-drawer__body padded by default with --flush opt-out. |
frontend/src/pages/WorkspaceDetail.jsx |
Migrates Workspace detail (including loading/not-found) to PageLayout. |
frontend/src/pages/WordPress.jsx |
Uses Drawer flush for a region-owning create flow. |
frontend/src/pages/TestSandbox.jsx |
Migrates TestSandbox to PageLayout. |
frontend/src/pages/Terminal.jsx |
Migrates Terminal/Logs to PageLayout fill and replaces slide-overs with ds/Drawer. |
frontend/src/pages/Templates.jsx |
Uses Drawer flush for install flow drawer. |
frontend/src/pages/Settings.jsx |
Migrates Settings to PageLayout. |
frontend/src/pages/Servers.jsx |
Uses Drawer flush for agent install drawer. |
frontend/src/pages/ProjectDetail.jsx |
Migrates Project detail (including loading/not-found) to PageLayout. |
frontend/src/pages/Notifications.jsx |
Migrates Notifications to PageLayout. |
frontend/src/pages/NotFound.jsx |
Migrates NotFound to PageLayout (including initial loading state). |
frontend/src/pages/MonitorDetail.jsx |
Migrates Monitor detail (including pre-load states) to PageLayout. |
frontend/src/pages/ImportWizard.jsx |
Migrates ImportWizard to PageLayout. |
frontend/src/pages/Git.jsx |
Migrates Git page (including loading branch) to PageLayout. |
frontend/src/pages/Email.jsx |
Migrates Email to PageLayout. |
frontend/src/pages/Documentation.jsx |
Migrates Documentation to PageLayout. |
frontend/src/pages/DeliveryLog.jsx |
Migrates DeliveryLog to PageLayout (including non-admin branch). |
frontend/src/pages/CronJobs.jsx |
Rebuilds cron create/edit into a Drawer using SchedulePicker and a crontab preview. |
frontend/src/pages/Coexistence.jsx |
Migrates Coexistence to PageLayout. |
frontend/src/pages/CloudflareZoneSettings.jsx |
Migrates Cloudflare zone settings to PageLayout while preserving detail-page chrome. |
frontend/src/pages/AppMap.jsx |
Migrates AppMap to PageLayout. |
frontend/src/layouts/PageLayout.jsx |
New standalone page shell wrapper around PageTopbar + scroll region. |
frontend/src/components/ds/Drawer.jsx |
Adds flush + bodyClassName and applies conditional body padding class. |
frontend/src/components/docker/ContainersTab.jsx |
Replaces custom container logs slide-over with ds/Drawer (flush). |
frontend/src/components/databases/EngineInstallDrawer.jsx |
Uses Drawer flush for two-pane/region-owning content. |
frontend/src/components/databases/EngineCatalogDrawer.jsx |
Uses Drawer flush for two-pane/region-owning content. |
frontend/src/components/dashboard/grid/WidgetLibrary.jsx |
Uses Drawer flush for region-owning widget library UI. |
frontend/src/components/dashboard/grid/WidgetEditor.jsx |
Uses Drawer flush for region-owning widget editor UI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+594
to
+597
| // `.sk-drawer__body` is the scroll region and carries no padding of its own | ||
| // (some drawers are edge-to-edge), so every drawer body pads itself. Same | ||
| // inset as .cron-drawer so the two cron drawers line up. | ||
| padding: $space-5 $space-5 $space-8; |
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.
Twenty-something pages had each been hand-assembling the same frame — a top bar, a scroll region, a centered reading column — and, unsurprisingly, twenty-something pages had drifted into slightly different answers. Some dropped their top bar into a padded container, which left the bar floating in the middle of the well with a gutter on all four sides; others reached for
:has()escape hatches to undo the padding they'd just inherited. This PR introducesPageLayout, the standalone-page twin of the existingTabGroupLayout, and moves those pages onto it so the frame comes from one place. The same consolidation happens one level down: the DSDrawerbody now pads itself by default (three different insets across ten drawers, and four bodies with no padding at all, is not a design system), and the last three hand-rolled slide-overs move onto the shared drawer so they finally get a focus trap and escape-to-close. On top of that, cron's create flow got a real rebuild — a bare cron text field and a preset dropdown became a drawer built on the sharedSchedulePicker, which incidentally revealed that the picker's stylesheet had never been imported at all.Highlights
Technical changes
frontend/src/layouts/PageLayout.jsx— rendersPageTopbarpinned to a full-bleed content region over a scrolling body, withfillto hand the body the whole region andtopbarClassName/contentClassNamefor pages that need to keep their own inner chrome._main-content.scssfactors the shell intopage-shell,page-shell-content, andpage-shell-innermixins shared by.sk-pageshell(standalone) and.sk-tabgroup(routed groups), including the$breakpoint-mdpadding step that was previously duplicated..sk-pageshell, not.sk-page—components/PageSkeleton.jsxalready owns that name for its loading archetypes, and itsgap: $space-4leaked in as a band of page background between the bar and the content on every shell page. The layout smoke test gained a vertical-gap assertion (content top must equal the bar's bottom edge) to catch a recurrence.PageLayout:AppMap,CloudflareZoneSettings,Coexistence,CronJobs,DeliveryLog,Documentation,Email,Git,ImportWizard,MonitorDetail,NotFound,Notifications,ProjectDetail,Settings,Terminal,TestSandbox,WorkspaceDetail.Git.jsxpreviously hand-rolledpage-container--full-bleed sk-tabgroupplus its own__content/__innerwrappers, duplicated across the loading branch and the main return; both branches now render through the layout.Terminal.jsxrenders infillmode;_terminal.scssdrops the0 $space-4 $space-4inset and the per-paneborder/border-radiusso.term-shell,.lv-page, and.proc-pagemeet the region edges..svc-pageis a scrolling card grid rather than a console pane, so it re-pads itself and keeps its ownoverflow-y._workspaces.scssdeletes the.main-content:has(.ws-detail-page)escape hatch — the shared full-bleed rule covers it, and a second:has()rule fought it at mobile widths over the header clearance.ds/Drawer.jsxgainsflushandbodyClassName;.sk-drawer__bodyis padded to the header's own18px 18px 28pxinset by default, with--flushzeroing it.flush:TemplatesandWordPress(form with a pinned footer),EngineCatalogDrawerandEngineInstallDrawer(two-pane catalog),WidgetEditor,WidgetLibrary, and theServersagent-install drawer. The now-duplicate per-drawer padding is deleted from_domains.scss,_monitors.scss(two rules), and_telemetry.scss.ContainersTab.jsxand bothTerminal.jsxdetail panels replace.preview-drawer+.preview-drawer-backdrop+ a hand-rolled close button withds/Drawer, picking up the Radix sheet's focus trap and escape handling. The File Manager preview is deliberately left alone (it also renders inline as a column pane, which a sheet can't do), as is Docker's container inspector (dx-*is the workspace's own language by design).CronJobs.jsxreplaces the create/editModalwith aCronFormDrawerbuilt onSchedulePicker. The drawer owns its form state and seeds onopenrather than mount (so it stays mounted for the slide-out and is reused for both modes), which lets the page drop thejobFormobject,resetForm,handleSubmitJob, and theapi.getCronPresets()fetch that only fed the old dropdown. New jobs default to0 3 * * *instead of an empty expression the picker would have to render as invalid.trim().replace(/\s+/g, ' ')), gates submit on name + command + schedule, guards against double-submit via asavingflag, and previews the resulting crontab line in a<pre>.main.scssimportscomponents/_schedule-picker— the stylesheet existed but was never wired in, soSchedulePickerrendered unstyled everywhere it was used, not just in cron.MonitorDetail,ProjectDetail, andWorkspaceDetailrender their loading and not-found branches through the same shell as the loaded page, eliminating the top-bar pop-in.