-
Notifications
You must be signed in to change notification settings - Fork 535
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
Fix #4027 #5208
base: main
Are you sure you want to change the base?
Fix #4027 #5208
Conversation
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
@@ -31,5 +32,5 @@ export function useResizeObserver<T extends HTMLElement>(callback: ResizeObserve | |||
return () => { | |||
observer.disconnect() | |||
} | |||
}, [target]) | |||
}, [targetEl]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for reviewer: Adding a ref as dependency isn't a recommended pattern as changes to the value of the ref does not cause the effect to re-run. Replaced the ref with targetEl instead
@@ -107,7 +107,7 @@ export const AnchoredOverlay: React.FC<React.PropsWithChildren<AnchoredOverlayPr | |||
overlayProps, | |||
focusTrapSettings, | |||
focusZoneSettings, | |||
side = 'outside-bottom', | |||
side = overlayProps?.['anchorSide'] || 'outside-bottom', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not do this just yet. Pull this out to another PR of it's own
@@ -45,7 +45,8 @@ export function useAnchoredPosition( | |||
|
|||
useLayoutEffect(updatePosition, [updatePosition]) | |||
|
|||
useResizeObserver(updatePosition) | |||
useResizeObserver(updatePosition) // watches for changes in window size | |||
useResizeObserver(updatePosition, floatingElementRef as React.RefObject<HTMLElement>) // watches for changes in floating element size |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note for self: This needs to be opt-in. A bit risky to always do this because it would keep jumping around.
Goal: Once SelectPanel opens upwards, do not resize it anymore.
Closes #
Changelog
New
Changed
Removed
Rollout strategy
Testing & Reviewing
Merge checklist