gluestack / gluestack-ui

React & React Native Components & Patterns (copy-paste components & patterns crafted with Tailwind CSS (NativeWind))
https://gluestack.io/
MIT License
2.32k stars 110 forks source link

Number of issues in useOverlayPosition #2104

Open juhasuni opened 4 months ago

juhasuni commented 4 months ago

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:

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.

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

There are other return props that could also be memoized.

gluestack-ui Version

@react-native-aria/overlays@0.3.12

Platform

Other Platform

Windows

Additional Information

No response

surajahmed commented 4 months ago

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