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

feat: added portalName, timeoutLength and ...rest props #1

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

soulfresh
Copy link

What does this PR do?

  • Added portalName to customize the PortalHost where popup content is
    placed.
  • Added timeoutLength to customize the delay when measuring popup content.
  • Pass ...rest props to the View wrapping popup content.

portalName

I ran into an issue where using Popup in a React Native Modal, covered the popup content. In order to work around that issue, I added a named PortalHost to my modal and passed that name through the Popup component.

timeoutLength

I was running into issues wher the popup content was sometimes positioning itself at 0, 0. The root issue was that the initial layout measurements were happening before the content was rendered. To solve this I wrapped that initial measureWindow request in a timeout similar to the existing timeout used with the Dimensions event listener. I further allowed customizing the timeout for all 3 measureWindow requests.

...rest

For accessibility on web, I wanted to combine Downshift.js with Popup. However, Downshift (and accessibility patterns on web), require Select, Tooltip, etc. contents to always be accessible in the DOM. I was mostly able to work around that by controlling the overlay prop. However, I also needed to configure the pointerEvents prop of the popup content so it would not block interactions below it. To resolve this, I apply any additional props to the View component wrapping the user's popup content.

I know this is a lot of changes for a single PR but I figured it would be easier for us both to handle everything in one PR rather than multiple. Let me know if you want me to break any of these changes out or remove any of them entirely.

- Added `portalName` to customize the `PortalHost` where popup content is
  placed.
- Added `timeoutLength` to customize the delay when measuring popup content.
- Pass `...rest` props to the `View` wrapping popup content.
height,
});
});
const timer = setTimeout(() => {
Copy link
Author

@soulfresh soulfresh Jul 28, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Despite adding this timeout, there are still some layout issues with the popover content. When opening the popup, it will jump around until it has finished calculating the optimal dimensions for the content. A bigger fix for that might be rendering the content transparently in the center of the screen and then placing/showing it after the content size is known.

- Fixed an issue where the anchorRef.current value is undefined in timeouts
  despite wrapping the timeout in an anchorRef.current check.
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

Successfully merging this pull request may close these issues.

None yet

1 participant