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

Number of issues in useOverlayPosition #2104

Open
3 of 6 tasks
juhasuni opened this issue Apr 30, 2024 · 1 comment
Open
3 of 6 tasks

Number of issues in useOverlayPosition #2104

juhasuni opened this issue Apr 30, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@juhasuni
Copy link

juhasuni commented Apr 30, 2024

Description

There are numerous issues in the implementation of useOverlayPosition hook. The hook also has significant differences across different platforms.

CodeSandbox/Snack link

No response

Steps to reproduce

useOverlayPosition hook is not consistent across different platform:

  • Web version accepts props containerPadding, boundaryElement, shouldUpdatePosition and onClose, which are not supported on other platforms (could be others as well)
  • APPROX_STATUSBAR_HEIGHT is undefined for all but "ios" and "android", causing issues e.g. if the platform is "windows"

Calling requestAnimationFrame causes infinite loop if either overlayOffset or triggerOffset truly has no dimensions:

https://github.com/gluestack/gluestack-ui/blob/055d69ac9399d978a1abedb5a47b9fb061d5a32e/packages/react-native-aria/overlays/src/useOverlayPosition.ts#L125C5-L133C6

State is being updated after component has been unmounted:

https://github.com/gluestack/gluestack-ui/blob/055d69ac9399d978a1abedb5a47b9fb061d5a32e/packages/react-native-aria/overlays/src/useOverlayPosition.ts#L158C3-L162C10

A new instance of updatePosition is always created making it inefficient to use id e.g. as a dependency of another hook as the dependency updates every time.

There are other return props that could also be memoized.

gluestack-ui Version

@react-native-aria/overlays@0.3.12

Platform

  • Expo
  • React Native CLI
  • Next
  • Web
  • Android
  • iOS

Other Platform

Windows

Additional Information

No response

@juhasuni juhasuni added the bug Something isn't working label Apr 30, 2024
@surajahmed
Copy link
Contributor

@juhasuni Thanks for reporting this. We'll have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants