Skip to content

fix(storybook): fill the app shell viewport - #3510

Merged
hqhq1025 merged 1 commit into
apache:mainfrom
hqhq1025:codex/storybook-shell-viewport
Aug 22, 2026
Merged

hqhq1025 merged 1 commit into
apache:mainfrom
hqhq1025:codex/storybook-shell-viewport

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

  • remove the inline percentage height that overrides the production 100dvh app-frame rule
  • let AppShell stories fill the Storybook iframe instead of ending at the 640px minimum and leaving a white band

Verification

  • npm --workspace @maka/desktop run typecheck:stories
  • visually checked the default AppShell story at 1200x768; the shell now fills the full viewport with no bottom white band

@hqhq1025

Copy link
Copy Markdown
Contributor Author

Visual comparison

Same 1200 × 768 viewport and application state for both captures.

Before

clipboard

After

clipboard

@hqhq1025
hqhq1025 requested review from Astro-Han and a lite review from Copilot August 22, 2026 15:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts the AppShell Storybook fixture so the shell frame can use the same 100dvh sizing rules as production, allowing the story to fill the Storybook iframe viewport (eliminating the bottom “white band” caused by the previous inline sizing override).

Changes:

  • Removed the inline height: '100%' on the AppShell story frame so .appFrame / .agents-layout-root CSS (height: 100dvh) can take effect.
  • Kept the existing minHeight: 640 guardrail while allowing the frame to grow to the full viewport height.

Required Conclusion (per code-review skill):

  1. Is the current solution optimal for the actual problem? Yes. The root cause is the inline height: '100%' overriding the CSS height: 100dvh on .appFrame/.agents-layout-root, and removing it is the smallest coherent fix at the source of truth (the CSS rules).
  2. If applicable, what production code can be deleted? none identified.
  3. If applicable, what low-quality tests can be deleted or replaced? none identified.
  4. Is a deeper refactor required, and what should the final structure be? No deeper refactor required; the story should rely on the same CSS sizing contracts as production rather than re-declaring height inline.
  5. Is the reviewed revision ready to merge? Yes.
  6. What residual risks or verification gaps remain? Minimal risk; change is isolated to Storybook presentation. No protected-area (security/licensing/release/governance) impact identified in this diff.

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@hqhq1025
hqhq1025 requested review from M4n5ter August 22, 2026 15:30

@jackwener jackwener left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Story layout is fullscreen; inner shell already uses height="fill". height: 100% on the frame is a percentage of an indefinite parent, so the story does not fill the canvas. Leaving minHeight: 640 is the right floor. Approve.

@hqhq1025
hqhq1025 merged commit 1579668 into apache:main Aug 22, 2026
1 check passed
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.

3 participants