Skip to content

fix(AppShell): content min-height in auto height mode - #604

Closed
cixzhang wants to merge 1 commit into
mainfrom
navi/fix/auto-height-scroll
Closed

fix(AppShell): content min-height in auto height mode#604
cixzhang wants to merge 1 commit into
mainfrom
navi/fix/auto-height-scroll

Conversation

@cixzhang

Copy link
Copy Markdown
Contributor

In auto height mode, the content area should fill at least the remaining viewport after the header. Currently short content leaves empty space below.

Adds minHeight: calc(100dvh - var(--appshell-header-height, 0px)) to the content area in auto mode, using the existing CSS variable that's already measured via ResizeObserver.

The sidenav sticky behavior in auto mode was already implemented. This ensures the content side also fills correctly.

Part of #565

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Mar 13, 2026
@github-actions

github-actions Bot commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

AppShell · View in Storybook
Metric Before After Delta
Bundle Size (ESM) N/A 625B N/A
Lines of Code N/A 389 -
Complexity N/A Very High (49) -

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@xds/core 9.4KB 15.1KB 2.4KB

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@cixzhang
cixzhang force-pushed the navi/fix/auto-height-scroll branch 2 times, most recently from b8788c7 to e8cffe6 Compare March 13, 2026 23:41
In auto height mode, the content area should fill at least the remaining
viewport height after the header. Adds min-height: calc(100dvh - header)
to the content area via the existing --appshell-header-height CSS variable.

The sidenav sticky behavior in auto mode was already implemented
(position: sticky + height: calc(100vh - header)). This fix ensures
the content area also respects the viewport minimum.

Part of #565
@cixzhang
cixzhang force-pushed the navi/fix/auto-height-scroll branch from e8cffe6 to 22c1228 Compare March 14, 2026 02:46
@cixzhang

Copy link
Copy Markdown
Contributor Author

Closing — the shell container already has minHeight: '100dvh' in auto mode via styles.rootAuto. The content area should fill naturally via flex (flex: 1). The real issue from #565 is the sidenav sticky behavior in auto mode, not the content min-height. Will rethink as part of the broader shell deep review.

@cixzhang cixzhang closed this Mar 14, 2026
cixzhang added a commit that referenced this pull request Mar 14, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
cixzhang added a commit that referenced this pull request Mar 14, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
cixzhang added a commit that referenced this pull request Mar 14, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
@cixzhang
cixzhang deleted the navi/fix/auto-height-scroll branch April 9, 2026 14:32
cixzhang added a commit that referenced this pull request Apr 26, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
cixzhang added a commit that referenced this pull request Jun 21, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
cixzhang added a commit that referenced this pull request Jun 21, 2026
Fix sidenav not filling height in auto mode and sticky elements
being transparent when content scrolls underneath.

- Sticky header and sidenav wrappers get opaque backgrounds based
  on the shell variant (not inherit — section variant has no
  intermediate background to inherit from)
- Sidenav sticky wrapper uses display:flex + flexDirection:column
  so the panel fills the container
- Panel gets flex:1 only in auto mode (via xstyle) to avoid
  breaking the row-based layout in fill mode
- Use 100dvh instead of 100vh for sticky sidenav height
- Remove duplicate role=navigation from LayoutPanel — XDSSideNav
  already provides the navigation landmark with aria-label

Fixes #604
Part of #565
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant