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

Popper's containerElement doesn't work correctly/support full screen API #978

Closed
NicholasBoll opened this issue Feb 23, 2021 · 1 comment · Fixed by #1403
Closed

Popper's containerElement doesn't work correctly/support full screen API #978

NicholasBoll opened this issue Feb 23, 2021 · 1 comment · Fixed by #1403
Assignees
Labels

Comments

@NicholasBoll
Copy link
Member

🐛 Bug Report

Since the introduction of the PopupStack, containerElement no longer works properly. The PopupStack works with only a single container (document.body) for coordinating popup stacking and z-indexes. Popper does render inside the containerElement, but all behaviors are based on document.body. This causes an issue with stack hooks like useCloseOnOutsideClick not detecting clicks inside the popup correctly.

A valid use-case is when using the Full Screen API. If the popup isn't a child of the full screen element, it will not be shown while in full screen mode. Full screen might be used for a video player.

To Reproduce

Set a containerElement in a Popper component and use useCloseOnOutsideClick. Inside clicks will not be detected correctly.

Link: https://codesandbox.io/s/color-picker-popup-custom-container-xvmmz
Steps:

  • click color picker icon
  • hold the mouse down on an item in the popup menu

Expected Behavior

The menu should not close

Actual Results

The menu closes. This prevents clicks from working properly in the menu.

Additional Notes

This will require changes to the PopupStack to allow for new popup stacks per container element. Popups still have to behave correctly relative to other items in the stack.

@NicholasBoll NicholasBoll added the bug Something isn't working label Feb 23, 2021
@divyanshu023
Copy link
Contributor

More Reproduction steps:
If the containerElement is not the root, the Popper is never able to reposition the contents properly. The contents of Popper does render inside containerElement, but Popper does not position them properly. While inspecting the dom, i found a phantom dom element attached to document.body which Popper was trying to position

https://codesandbox.io/s/nifty-forest-i2ys3?file=/src/index.tsx

@jpante jpante added the p:2 label Mar 1, 2021
@jpante jpante added enhancement New feature or request 5.x and removed bug Something isn't working labels Aug 9, 2021
@myvuuu myvuuu added p:1 and removed p:2 labels Oct 19, 2021
@myvuuu myvuuu added the s:5 label Nov 30, 2021
@NicholasBoll NicholasBoll self-assigned this Dec 9, 2021
@NicholasBoll NicholasBoll linked a pull request Jan 3, 2022 that will close this issue
@myvuuu myvuuu added 6.x and removed 5.x labels Jan 7, 2022
@jaclynjessup jaclynjessup moved this from 🆕 New to ✅ Done in Canvas Kit Aug 5, 2022
@jaclynjessup jaclynjessup moved this to 🆕 New in Canvas Kit Aug 5, 2022
@jaclynjessup jaclynjessup removed the enhancement New feature or request label Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants