microsoft / fluentui

Fluent UI web represents a collection of utilities, React components, and web components for building web applications.
https://react.fluentui.dev
Other
18.05k stars 2.69k forks source link

[Bug]: Drawer overlays entire screen instead of area it's placed in #29132

Open zachbryant opened 9 months ago

zachbryant commented 9 months ago

Library

React Components / v9 (@fluentui/react-components)

System Info

System:
    OS: Windows 10 10.0.22621 (edit: env info is wrong, I'm on win 11)
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 15.75 GB / 31.60 GB
  Browsers:
    Chrome: 116.0.5845.180
    Edge: Spartan (44.22621.2283.0), Chromium (116.0.1938.76)
    Internet Explorer: 11.0.22621.1

Are you reporting Accessibility issue?

no

Reproduction

https://react.fluentui.dev/?path=/docs/preview-components-drawer--default#default

Bug Description

Actual Behavior

Drawer overlays entire screen when in overlay mode. Covers content not belonging to its container.

Expected Behavior

Drawer should cover content belonging only to its container. In Office we have controls that can't be hidden by a drawer like this.

Logs

No response

Requested priority

Normal

Products/sites affected

Office.com

Are you willing to submit a PR to fix?

no

Validations

layershifter commented 9 months ago

@zachbryant are you looking for inline Drawer (https://react.fluentui.dev/?path=/docs/preview-components-drawer--default#inline)?

zachbryant commented 9 months ago

@layershifter unforunately not, as the inline shifts the entire layout to the side. Our use case dictates the drawer not going beyond a certain div, while also not shifting the main layout when open

layershifter commented 9 months ago

@marcosmoura can you please check this one?

thesharifi commented 9 months ago

Hey, @layershifter, I am interested in working on this repo, please assign this to me.

Weffe commented 5 months ago

We have the same constraints in Azure Data Explorer where the OverlayDrawer shouldn't cover the entire app (InlineDrawer is undesirable due to the content shift). In Fluent 8, I know the Panel component let you specify a hostId for the Layer it's rendered into. We'd need something like that again but for Drawer.

Weffe commented 5 months ago

It would be a neat idea that the Drawer pulls the target element it renders into from Context that's maybe stored on the FluentProvider. Or even an entirely separate Context the Drawer consumes. Either way, we could skip needing to set the target element every time we render a Drawer by setting it once on the Context Provider (although that's easily avoided by wrapping Drawer with our own customization). Of course, we could always accept the target element as a prop too if we need to do it as a one-off.

Something like this:

// DrawerTargetContainerContext.tsx
const DrawerTargetContainerContext = React.createContext<null | HTMLElement>(null);
const useDrawerTargetContainerContext = () => React.useContext(DrawerTargetContainerContext);

// Drawer.tsx
itnerface DrawerProps {
  targetContainer?: HTMLElement;
}

const Drawer = (props: DrawerProps) => {
  const { targetContainer: targetContainerProp } = props;

  // Fluent provider
  const fluentProvider = useFluentProvider();
  const fluentTargetContainer = fluentProvider.drawerTargetContainer;
  const targetContainer = targetContainerProp  ?? fluentTargetContainer ?? window.document.body; 

  // DrawerTargetContainerContext 
  const targetContainerContext = useDrawerTargetContainerContext();
  const targetContainer = targetContainerProp  ?? targetContainerContext ?? window.document.body;

  // Render into targetContainer
}

Edit: I see now that the Drawer has a mountNode that we can use. But, the issue is with the styling for the Drawer being position: fixed and not position: absolute to respect the parent container's size. Otherwise, it's always going to take up the full height of the viewport even if the parent was height: 500px for example.

This bug issue also applies to the Dialog since Drawer extends off of it.