Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use parent overlay as default container for child overlays #1003

Open
dleavitt opened this issue May 18, 2022 · 0 comments
Open

Use parent overlay as default container for child overlays #1003

dleavitt opened this issue May 18, 2022 · 0 comments

Comments

@dleavitt
Copy link

Is your feature request related to a problem? Please describe

Overlays, when not passed an explicit container prop, attach themselves to document.body. This doesn't work well if you nest overlays as the child overlay isn't actually inside the parent.

Specifically:

  1. If you've got an overlay with rootClose that contains another overlay, interacting with the inner overlay will close the outer overlay.
  2. If you have an overlay inside a modal you can't focus on the contents of the overlay, since they're attached to the root and outside the modal.

Repro (with bs5 react-bootstrap, also affects bs4):
https://codesandbox.io/s/great-montalcini-s2yebc?file=/src/App.js

See:

Describe the solution you'd like

When an overlay is inside of another overlay, default its container to a child of the parent overlay instead of to document.body.

This would be accomplished with React context. Each overlay:

  1. Looks in context for a parent overlay ref and uses that as its default container if present.*
  2. Wraps its children in a provider that passes a ref to itself as the parent overlay ref to be used by its children.

*Except modals, which should always be attached to document.body.

Roughly like this:

const OverlayContext = createContext(null)

const Overlay = () => {
  // ... overlay stuff

  // grab parent ref from overlay context as the default container
  const containerRef = useContext<OverlayContext>()
  container ||= containerRef

  // ... more overlay stuff

  const overlayRef = useWaitForDOMRef(outerRef) // ish
  // inject ref of the overlay into context for any children
  <OverlayContext.Provider value={overlayRef}>
    {child}
  </OverlayContext.Provider>
}

Describe alternatives you've considered

  • Set enforceFocus to false on modals. This avoids the second problem but at the cost of allowing focus on things that are actually outside the modal, which sort of throws the baby out with the bath water.
  • Explicitly set the container prop on all overlays. This breaks composability - how is the overlay to know whether it's inside a modal or not?
  • Do the above solution with an OverlayContext in the application code. This requires either either adding a bunch of boilerplate inside and outside of every modal/overlay, or making a wrapper for each of the overlay-based components in react-bootstrap.

Additional context

Perhaps you are wondering "what sort of monster would want to nest overlays like this?" A reasonable question! One specific place this has come up is inline UI for filtering a table. Here's an example of such an element from material UI:

Screen Shot 2022-05-18 at 13 06 19

Here we've got a popover-esque with controls inside of it. The controls may themselves have overlays (popovers or multi-pickers) or the popover and table may be inside a modal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant