Skip to content

Give App Shell and Document Layout regions the padding/background/border they never had - #23

Open
KolesnikovKirill wants to merge 1 commit into
mainfrom
fix/app-shell-document-layout-region-chrome
Open

KolesnikovKirill wants to merge 1 commit into
mainfrom
fix/app-shell-document-layout-region-chrome

Conversation

@KolesnikovKirill

Copy link
Copy Markdown
Member

What and why

You pointed at a real screenshot: App Shell's sidebar and Document Layout's sidebar/aside had their content sitting flush against the edge, no visible panel around it at all.

Checked why rather than guessing at a fix — .cm-app-shell__sidebar, __aside, __content, __header, __subheader, and __footer, and the equivalent five regions in document-layout.css, had never had padding, and the sidebar/aside additionally never had a background or border. This predates #22 (just merged): that PR fixed positioning, responsiveness, and sticky behavior for both files but left this specific gap, because I verified those changes by checking for console warnings and correct attribute/behavior state, not by looking hard enough at what was actually on screen.

What changed

VueForge's fd793696 had exactly this chrome: .vf-sidebar-area / .vf-aside-area carried sidebarBackground/sidebarBorder and asidePaddingBlock/Inline, and .vf-content-area carried its own padding. Added the same shape here, using this package's own tokens rather than porting VueForge's token names:

  • --cm-space-3 padding and --cm-color-background-surface on sidebar/aside/content
  • border-inline-end on the sidebar and border-inline-start on the aside (--cm-color-border-subtle, matching every other border already in both files)
  • --cm-space-2 --cm-space-3 padding on header/subheader/footer, matching the padding shorthand admin-layout.css and admin-shell.css already use for their own header/footer

Verified against the real thing

Screenshotted both layouts after rebuilding, at the viewport width from your screenshot, and confirmed sidebar/aside now render as distinct panels with visible backgrounds and borders, with their slot content no longer touching the edges.

Checks

npm run verify passes in full.

…der they never had

You pointed at a real screenshot: App Shell's sidebar and Document Layout's sidebar/aside had
their content sitting flush against the edge, no visible panel around it at all. Checked why rather
than guessing at a fix -- .cm-app-shell__sidebar, __aside, __content, __header, __subheader, and
__footer, and the equivalent five regions in document-layout.css, had never had padding, and the
sidebar/aside additionally never had a background or border. This predates the fidelity-restoration
PR that just merged (#22): that PR fixed positioning, responsiveness, and sticky behavior for both
files but left this specific gap, because I verified those changes by checking for console warnings
and correct attribute/behavior state, not by looking hard enough at what was actually on screen.

VueForge's fd79369 had exactly this chrome: .vf-sidebar-area / .vf-aside-area carried
sidebarBackground/sidebarBorder and asidePaddingBlock/Inline, and .vf-content-area carried its own
padding. Added the same shape here, using this package's own tokens rather than porting VueForge's
token names: --cm-space-3 padding and --cm-color-background-surface on sidebar/aside/content, with
border-inline-end on the sidebar and border-inline-start on the aside (--cm-color-border-subtle,
matching every other border already in both files); --cm-space-2 --cm-space-3 padding on
header/subheader/footer, matching the padding shorthand admin-layout.css and admin-shell.css
already use for their own header/footer.

Verified live against the real dev server, not by reading the diff: screenshotted both layouts
after rebuilding, at the viewport width from your screenshot, and confirmed sidebar/aside now
render as distinct panels with visible backgrounds and borders, with their slot content no longer
touching the edges.

npm run verify passes in full.
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