[dialog] Opening dialog overrides padding on the body element

[SiTaggart]

🐛 Bug report

Current Behavior

When the dialog is open, react-remove-scroll via react-remove-scroll-bar, sets some css properties on the body. One of them is padding-top which is set to 0. We need to not do that for our page layout to not break.

We use a sticky masthead which the underlying content uses a padding top value on the body to not be obscured by the masthead.

When react-remove-scroll-bar removed the top padding, the top of the content is under the masthead and the bottom of the page jumps up.

https://share.getcloudapp.com/JruLNJ1j

Expected behavior

react-remove-scroll-bar does allow you to prevent or override some of these css properties. I should be able to pass dialog some control to prevent this from happening.

Suggested solution(s)

expose some react-remove-scroll-bar props to override the behavior.

Your environment

https://www.twilio.com/console/iot/supersim/network-access-profiles

[chaance]

I'm not sure how simply we can override this.

• Padding styles are added in react-remove-scroll-bar (https://github.com/theKashey/react-remove-scroll-bar/blob/master/src/component.tsx#L29), which is then used in...
• ...react-remove-scroll (https://github.com/theKashey/react-remove-scroll/blob/fc579abfd4d5a422ce8ad6ccb208e00dbe5dd3d6/src/SideEffect.tsx#L248) which is what we use. All of those style overrides exist in the former, but they are inaccessible in the latter. It seems like gapMode might help here but we can't pass that prop down AFAICT.

@theKashey Any suggestions on your end?

[theKashey]

So that's a really hard part. I've spend quite a lot of time trying to understand that actually shall be done and "why", and at the end I didn't found an answer, that's why gapMode is still configurable.
Frankly speaking - all this voodoo magic is not required in case of allowRelative, but there are very good cases for allowRelative and it's enabled by default.
Probably adding logic "if !allowRelative don't touch padding-top", but that's would not end well.

And actually I have a simple suggestion - In order to have a gap between browser-top and the page content - don't use paddings on the body, but add a <div style="height:yourPadding"/> and it would result in the picture. Ie use additinal html tag instead of style on the body.

[SiTaggart]

In order to have a gap between browser-top and the page content - don't use paddings on the body, but add a

and it would result in the picture. Ie use additinal html tag instead of style on the body.

Ordinarily this would be a fine solution. Something I would actually prefer honestly, but we are battling with previous folks decisions and almost zero confidence in being able to make that change to the application body styles.

Is there no way I can affect the behavior of the various remove-scroll packages from the dialog?

[theKashey]

You can always add !important to your own styles or overspecifitify them in any other way. However that might shift your absolute positioned elements this time. Well, if you have them.

Another, probably good and working solution is to move existing styles "as-is" from the body to react root. Padding would be still "above your components" and effect in the same way, shifting your content a bit down, but it will not conflict with any other styles on the body.

[ryparker]

Just sharing my workaround here in case anyone has a need to override this as well.

I passed dangerouslyBypassScrollLock into the DialogOverlay and used the useLockBodyScroll() hook from usehooks.com

i.g. implementation

const LoginDialog = (props: LoginDialogProps) => {
  const { onDismiss, ...delegated } = props;
  useLockBodyScroll();
  return (
    <DialogOverlay onDismiss={onDismiss} dangerouslyBypassScrollLock {...delegated}>
      <ContentBox aria-label="sign in dialog">
        <Title>Sign in</Title>
        <Button>Focusable</Button>
      </ContentBox>
    </DialogOverlay>
  );
};

[theKashey]

Unfortunately useLockBodyScroll is not preventing scroll on Safari.
Wondering if this is related:

• feat: use css vars with fallbacks theKashey/react-remove-scroll-bar#30
• Better explanation of how it should work theKashey/react-remove-scroll-bar#31

[ryparker]

@theKashey Good catch, I found a better solution from usehooks-ts.com. This works on Safari (desktop & mobile).

Here's a demo of it being used on https://congress.wiki/hr1-117

CleanShot.2022-05-19.at.09.57.32.mp4

Implementation:

const LoginDialog = (props: LoginDialogProps) => {
  const { isOpen, onDismiss, ...delegated } = props;

  // Lock body scroll when open
  const [locked, setLocked] = useLockedBody();
  useEffect(() => {
    setLocked(!!isOpen);
  }, [isOpen]);

  return (
    <DialogOverlay onDismiss={onDismiss} dangerouslyBypassScrollLock {...delegated}>
      <ContentBox aria-label="sign in dialog">
        <Title>Sign in</Title>
        <Button>Focusable</Button>
      </ContentBox>
    </DialogOverlay>
  );
};