Skip to content

[drawer] Forward viewport style prop#4841

Merged
atomiks merged 1 commit into
mui:masterfrom
atomiks:codex/drawer-viewport-style
May 18, 2026
Merged

[drawer] Forward viewport style prop#4841
atomiks merged 1 commit into
mui:masterfrom
atomiks:codex/drawer-viewport-style

Conversation

@atomiks
Copy link
Copy Markdown
Contributor

@atomiks atomiks commented May 18, 2026

Drawer.Viewport now forwards the style prop to the rendered viewport, matching the rest of the component API and keeping conformance coverage in place.

Root cause

Drawer.Viewport destructured style but never passed it to DialogViewport, so styles set on the component were dropped.

Changes

  • Forward style from Drawer.Viewport to DialogViewport.
  • Add the standard conformance suite for Drawer.Viewport.

@atomiks atomiks added type: bug It doesn't behave as expected. component: drawer Changes related to the drawer component. labels May 18, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 18, 2026

commit: bc5e04b

@code-infra-dashboard
Copy link
Copy Markdown

code-infra-dashboard Bot commented May 18, 2026

Bundle size

Bundle Parsed size Gzip size
@base-ui/react 🔺+8B(0.00%) 0B(0.00%)

Details of bundle changes

Performance

Total duration: 1,075.90 ms -144.13 ms(-11.8%) | Renders: 50 (+0) | Paint: 1,648.05 ms -237.49 ms(-12.6%)

Test Duration Renders
Menu open (500 items) 59.43 ms ▼-16.18 ms(-21.4%) 12 (+0)

11 tests within noise — details


Check out the code infra dashboard for more information about this PR.

@netlify
Copy link
Copy Markdown

netlify Bot commented May 18, 2026

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit bc5e04b
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/6a0b03162a9c30000811748d
😎 Deploy Preview https://deploy-preview-4841--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@atomiks atomiks marked this pull request as ready for review May 18, 2026 12:20
@atomiks
Copy link
Copy Markdown
Contributor Author

atomiks commented May 18, 2026

Code Review: #4841 — [drawer] Forward viewport style prop

Overview

Fixes a bug where Drawer.Viewport destructured style out of its props but never forwarded it to the underlying DialogViewport — so any style set on Drawer.Viewport was silently dropped. Also adds the standard describeConformance suite for the component (13 additions, 1 deletion).

Verification performed

  • Root cause confirmed. DrawerViewport.tsx:70 destructures style into a local variable, which excludes it from ...elementProps. Before the fix, style reached neither DialogViewport directly nor the spread elementProps, so it was lost. render, className, and children were already forwarded — style was the lone omission, so "matching the rest of the component API" is accurate.
  • Fix is correct. DialogViewport accepts style via BaseUIComponentProps<'divّ> and resolves it in useRenderElementPropsresolveStyle(styleProp, state) then mergeObjects(outProps.style, style) (useRenderElement.tsx:106). The user style merges with DialogViewport's internal pointerEvents style, with user style winning on conflict — consistent with DialogViewport's own behavior.
  • Tests pass. Ran pnpm test:jsdom DrawerViewport — all 50 pass (6 browser-only skipped).
  • Regression test is genuine. Reverted just the one-line source fix and re-ran: the conformance test forwards the custom 'style' attribute defined on the component fails. So the new conformance suite catches this exact bug.

Code quality & conventions

  • The fix mirrors the className={className} line directly above it — minimal and idiomatic.
  • The conformance setup correctly follows the drawer convention (<Drawer.Root open><Drawer.Portal> — same as DrawerPopup.test.tsx and DrawerContent.test.tsx), rather than copying the Dialog.Viewport pattern (modal={false} + nested <Dialog.Popup />). Drawers default to modal, so this is the right call.
  • Commit message follows [scope] Imperative summary.

Minor notes (non-blocking)

  • Function-form style ((state) => CSSProperties) is now forwarded too, and DialogViewport resolves it against DialogViewportState. DrawerViewportState and DialogViewportState are structurally identical, and this is the same path the already-forwarded function-form className takes — so no issue, just worth being aware the function receives the dialog-typed state.
  • The conformance test renders an empty Drawer.Viewport (no Drawer.Popup child). Conformance only exercises the viewport element itself, so this is fine and all tests pass — adding a popup child would be marginally more realistic but isn't needed.

Risks

Effectively none. The change is purely additive — it restores a previously-dropped prop. No security, performance, or API-surface concerns.

Verdict: Looks good to merge. Small, correct, well-tested bug fix with a verified regression test.

@atomiks atomiks merged commit 5579e7f into mui:master May 18, 2026
22 of 24 checks passed
@atomiks atomiks deleted the codex/drawer-viewport-style branch May 18, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: drawer Changes related to the drawer component. type: bug It doesn't behave as expected.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant