Skip to content

Conversation

@michaldudak
Copy link
Member

@michaldudak michaldudak commented Jan 13, 2025

@michaldudak michaldudak added the component: menu Changes related to the menu component. label Jan 13, 2025
@netlify
Copy link

netlify bot commented Jan 13, 2025

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 8e1df4f
🔍 Latest deploy log https://app.netlify.com/sites/base-ui/deploys/6792469f7398290008ff6d18
😎 Deploy Preview https://deploy-preview-1330--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@atomiks
Copy link
Contributor

atomiks commented Jan 14, 2025

When closing the menu shifts up and to the left by ~5-10px, causing a jitter?

@michaldudak
Copy link
Member Author

michaldudak commented Jan 22, 2025

When closing the menu shifts up and to the left by ~5-10px, causing a jitter?

This is actually an effect of another issue: when useScrollLock is applied, it changes the body styles setting position: relative and box-sizing: border-box. The experiment page uses default styles for the body, so when the scroll lock is removed, the reference for the menu's positioner changes, causing the shift. This might be less of an issue in real applications, as most often, default margin/padding will be removed from the html and body elements (@colmtuite, correct me if I'm wrong).

I'll style the body element to fix the issue here.

For the record - this is how it currently looks:

Screen.Recording.2025-01-22.at.16.15.10.mov

@atomiks
Copy link
Contributor

atomiks commented Jan 23, 2025

This is actually an effect of another issue: when useScrollLock is applied

In order to fix #1343 on the body easily, we need to use mounted not open state to apply the scroll lock. That would also prevent this from being visible?

@michaldudak
Copy link
Member Author

It would prevent this exact problem, but all the other absolutely positioned elements on the page that only have statically positioned parents will be affected (they will shift when a popup opens and after it disappears), as in https://codesandbox.io/p/sandbox/headless-cloud-8s744f?file=%2Fsrc%2FApp.tsx%3A11%2C65.

@michaldudak
Copy link
Member Author

michaldudak commented Jan 23, 2025

One option to prevent this from happening would be to teach developers to set up a positioning context on the .Root element (add position: relative to styles.css in https://base-ui.com/react/overview/quick-start#set-up-portals). We can set position: relative on default portal containers ourselves.

@michaldudak michaldudak marked this pull request as ready for review January 23, 2025 11:02
@netlify
Copy link

netlify bot commented Jan 30, 2025

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 51808a5
🔍 Latest deploy log https://app.netlify.com/sites/base-ui/deploys/67a3656d007bf5000886ecf9
😎 Deploy Preview https://deploy-preview-1330--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@atomiks
Copy link
Contributor

atomiks commented Jan 31, 2025

Not super important, but the settings panel at the top left doesn't seem to be pre-rendered/SSR'd, and causes a layout shift on load

@michaldudak
Copy link
Member Author

It's defined in the experiment itself and portalled into the sidebar in the effect phase - that's why it appears slightly later.

@colmtuite
Copy link
Contributor

colmtuite commented Feb 3, 2025

Add position: fixed to the sidebar so it doesn't scroll with the main window.

@michaldudak michaldudak merged commit 53e01ad into mui:master Feb 5, 2025
23 checks passed
@michaldudak michaldudak deleted the experiments/menu-fully-featured branch February 5, 2025 13:30
@oliviertassinari oliviertassinari added internal Behind-the-scenes enhancement. Formerly called “core”. and removed core labels Aug 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: menu Changes related to the menu component. internal Behind-the-scenes enhancement. Formerly called “core”.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants