react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

Missing `updatePosition` from `usePopover` return value.

Open tgelu opened this issue 2 years ago โ€ข 4 comments

๐Ÿ› Bug Report

usePopover is meant to replace the low-lever useOverlayPosition but because of a missing return value it can't in some cases.

๐Ÿค” Expected Behavior

updatePosition should be returned from usePopover.

๐Ÿ˜ฏ Current Behavior

updatePosition from useOverlayPosition which is useful for many scenarios, is not returned by usePopover.

๐Ÿ’ Possible Solution

There's an old, open PR.

๐Ÿ”ฆ Context

Was looking at replacing our usage of low-level hooks which have been replaced by usePopover.

๐Ÿ’ป Code Sample

N/A

๐ŸŒ Your Environment

N/A

๐Ÿงข Your Company/Team

N/A

๐Ÿ•ท Tracking Issue (optional)

N/A

tgelu avatar Feb 10 '23 13:02 tgelu

What are you using updatePosition for?

devongovett avatar Feb 10 '23 13:02 devongovett

In two places:

  • a popover opened in the first render in controlled mode is not well positioned, so we updatePosition in a useLayoutEffect in such cases
  • for some reason, when a popover first opens on a touch devices, the position is not set, so we manually call updatePosition (it may have something to do with press handler invocation timing)

But I don't think we should focus too much on my use case. I guess it's only "coincidental" that I personally need updatePosition. I think the fact that updatePosition is returned from useOverlayPosition and the higher-level hook usePopover is meant to replace most, if not all, usages of useOverlayPosition it follows that updatePosition should also be returned from usePopover. Otherwise it is bound to still keep people on the lower-level hooks for various reasons (which may differ than the two examples above) and that defeats the purpose of usePopover for at least a chunk of react-aria users.

tgelu avatar Feb 13 '23 08:02 tgelu

@devongovett I could also really use this feature. We have a context menu which is tied to a field, and our designers want to be able to keep the menu open during a resize event. edit - triggering window resize event does the update but seems like an odd workaround @snowystinger cc

snow893 avatar Apr 27 '23 00:04 snow893

We show a document which can be zoomed and while zooming the document, we want to update the position of popover. This will be very helpful in that situation. Happy to open a PR.

ritz078 avatar Feb 10 '25 13:02 ritz078