DropdownMenu changes padding / margin on document body.

[steveruizok]

Bug report

Current Behavior

When active, the DropdownMenu component adds the following CSS to the document body.

  body {
    overflow: hidden !important;
    position: relative !important;
    padding-left: 8px;
    padding-top: 8px;
    padding-right: 8px;
    margin-left:0;
    margin-top:0;
    margin-right: 0px !important;
  }

This can cause layout shift on layouts that have set padding / margin on the body.

Expected behavior

Opening the dropdown should not cause the layout to shift.

Reproducible example

Codesandbox.

Suggested solution

I'm guessing that the margins / padding have a function, though it isn't clear to me what that is. As long as that part of the CSS may be removed without breaking the component, I would suggest removing the padding / margins.

Your environment

Software	Name(s)	Version
Radix Package(s)	Dropdown	0.0.1
React	n/a	17.0.0
Browser	Chrome
Assistive tech	n/a
Node	n/a
npm/yarn
Operating System	Mac

[raunofreiberg]

I'm fairly certain that this comes from the @radix/react-menu package using react-remove-scroll which removes the document scrollbar (as a result of an overlay being open) while maintaining its space.

As a workaround, don't place your padding or margin on the body element.

[steveruizok]

Noted, though even with a project where the body has no padding / margin, this will add padding and cause a layout shift.

[chaance]

It shouldn't cause a layout shift, in fact it's designed to prevent it. But you do need to set margin: 0 on the body, and as @raunofreiberg mentioned it's generally a good idea to avoid adding margin and padding directly to the body and instead using an inner element for that. Here's a fork of your sandbox showing that in action: https://codesandbox.io/s/damp-snow-ev0rd?file=/src/App.tsx

That said, we probably need to add documentation to clear this up if we don't have it already. If you still have layout shift after making those adjustment, let me know and we can definitely investigate further.

[steveruizok]

Right on, so to boost that for anyone coming here in the future, adding margin: 0; to body fixes the problem. 👍

body {
  margin: 0;
}

[pacocoursey]

Just me or has this broken again? I'm seeing weird behavior – either the Dropdown (or Context) menus are wayy too low from the trigger, or opening/closing the Menu causes the entire page to jump.

I bumped all the packages in Chance's example above and you can see the offset problem: https://codesandbox.io/s/admiring-brown-dn7e7?file=/src/App.tsx

Could this be re-opened?

[pacocoursey]

Ping @benoitgrelard and @jjenzz since you may not be following this issue already

[benoitgrelard]

@pacocoursey yes this is such a frustrating issue.

It's kind of a compound issue:

• as @raunofreiberg mentioned, we use react-remove-scroll which does a bunch of things we can't control. One of these is it sets position: relative on the body (We're hoping to soon take control of this by creating our own solution which hopefully would give us more control)
• our Popper solution doesn't support parent static contexts as we mostly rely on portalling (Eventually, we should add support for this, but so far it hasn't been a priority)
• with a basic setup like in your sandbox (even though you have padding: 0, margin: 0 on body), body will not start at top: 0 because of collapsing margins as you can see below:

So the extra gap you're seeing is because of all that.
There are a few ways to get around it but the gist of it is you want to get rid of the collapsing margins on that parent.

The easiest way would be to add style={{ margin: -1, padding: 1 }} to your top-level app like so:
https://codesandbox.io/s/dropdownmenu-corrected-collapsing-margins-iiop9

[steveruizok]

• as @raunofreiberg mentioned, we use react-remove-scroll which does a bunch of things we can't control. One of these is it sets position: relative on the body (We're hoping to soon take control of this by creating our own solution which hopefully would give us more control)

Hey, coming back to this as it's causing a strange layout issue for me. Any element with an absolute position that is taking its height as a percentage of the body will be thrown off once the body's position changes to relative.

Here's a repro: https://codesandbox.io/s/body-relative-issue-zz4q3?file=/App.js

In my case, I'm able to set the size as fixed, which solves the problem, but I can imagine cases where it would cause issues.

[upq]

It adds a margin-right: 17px !important to the body, so unless you override that using important inline styles it will never work .. I don't know why is this closed anyway ..

[jjenzz]

adding the following styles to your body should prevent issues i believe:

body {
  display: flow-root;
  min-height: 100vh;
}

[benoitgrelard]

Or the solution provided above: #346 (comment)

[jjenzz]

@benoitgrelard that doesn't solve @steveruizok's recent example unfortunately. the snippet i posted should work for all cases i believe.

[mithleshjs]

adding the following styles to your body should prevent issues i believe:

body {
  display: flow-root;
  min-height: 100vh;
}

@jjenzz It is not working as display: flow-root; gets replaced by display: relative !important;
None of the above solutions is working right now except wrapping the whole content in fixed div.