useOverlayPosition: strange behaviour of shouldUpdatePosition
- Dominant language
- TypeScript
- Stars
- 15.9k
- Forks
- 1.6k
- Avg merge
- 3d 9m
- Merged PRs (30d)
- 59
Description
Hey!
I think that the naming of `shouldUpdatePosition` is a little bit confusing. As I imagine, it should calculate position once and then ignore all resizes, scrolls, etc on subsequent updates
But according to line:
https://github.com/adobe/react-spectrum/blob/273e828cd17c3d229e8ab10ce62bd0a6f0f43963/packages/%40react-aria/overlays/src/useOverlayPosition.ts#L128
If shouldUpdatePosition is false, it will never calculate position even for initial render. I believe it should work like `eventListeners` modifier of Popper.js (https://popper.js.org/docs/v2/modifiers/event-listeners/)
This behaviour leads bug/behaviour like this:
https://codesandbox.io/s/great-mendel-l0c8ye?file=/src/App.js
wrong initial positioning with shouldUpdatePosition=false, since useOverlayPosition does nothing useful in this case
If you're agree with it and it won't break anything, let me submit a PR where positioning is calculated on first render anyway and event listeners are added only when shouldUpdatePosition=true :)
Contributor guide
Research direction
Start in packages/@react-aria/overlays/src/useOverlayPosition.ts at the referenced line 128, then reproduce the initial-positioning problem with the linked CodeSandbox. Verify the intended behavior for shouldUpdatePosition=false: initial positioning should still occur, while later resize and scroll updates should be controlled by that option.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- react, typescript
- Domain
- frontend
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100