-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[Popover] Reposition when rerender #10595
Comments
@KevinAsher We have added an For instance: function MyPopover() {
const [itemCount, setItemCount] = React.useState(0);
const popoverActions = React.useRef();
React.useEffect(() => {
setTimeout(() => setItemCount(100), 1000);
}, []);
React.useEffect(() => {
if (popoverActions.current) {
popoverActions.current.updatePosition();
}
}, [itemCount]);
const items = [];
for (let i = 0; i < itemCount; i++) {
items.push(<p key={i}>item</p>);
}
return <Popover action={popoverActions}>{items}</Popover>;
} However, it might require too much boilerplate, a workaround is to trigger a window resize event (don't abuse it!): window.dispatchEvent(new CustomEvent('resize')) In the future, we should consider using the ResizeObserver API for out of the box support:
Alternatively, we can consider updating the position at each render. It could be a low hanging fruit :). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
We should be able to apply a similar fix than #18310 (comment). I would propose something close to: diff --git a/packages/material-ui/src/Popover/Popover.js b/packages/material-ui/src/Popover/Popover.js
index 9f23c25e2..722ee83b6 100644
--- a/packages/material-ui/src/Popover/Popover.js
+++ b/packages/material-ui/src/Popover/Popover.js
@@ -335,32 +335,40 @@ const Popover = React.forwardRef(function Popover(props, ref) {
paperRef.current = ReactDOM.findDOMNode(instance);
}, []);
- const updatePosition = React.useMemo(() => {
- if (!open) {
- return undefined;
- }
-
- return debounce(() => {
+ React.useEffect(() => {
+ if (open && paperRef.current) {
setPositioningStyles(paperRef.current);
- });
- }, [open, setPositioningStyles]);
+ }
+ });
- React.useImperativeHandle(action, () => (open ? { updatePosition } : null), [
- open,
- updatePosition,
- ]);
+ React.useImperativeHandle(
+ action,
+ () =>
+ open
+ ? {
+ updatePosition: () => {
+ setPositioningStyles(paperRef.current);
+ },
+ }
+ : null,
+ [open, setPositioningStyles],
+ );
React.useEffect(() => {
- if (!updatePosition) {
+ if (!open) {
return undefined;
}
- window.addEventListener('resize', updatePosition);
+ const handleResize = debounce(() => {
+ setPositioningStyles(paperRef.current);
+ });
+
+ window.addEventListener('resize', handleResize);
return () => {
- window.removeEventListener('resize', updatePosition);
- updatePosition.clear();
+ handleResize.clear();
+ window.removeEventListener('resize', handleResize);
};
- }, [updatePosition]);
+ }, [open, setPositioningStyles]);
let transitionDuration = transitionDurationProp; Does somebody want to work on a pull request? :) |
Hello @oliviertassinari can I start to work on this issue? :) |
@SandraMarcelaHerreraArriaga No permission needed. Thanks for working on it! |
Expected Behavior
If the
<Select>
component is opened, and we are loading its items dynamically, it's expected that when the items are finally loaded the<Select>
's opened overlay adjusts itself to the current available space.Current Behavior
The
<Select>
's overlay overflows.Steps to Reproduce (for bugs)
In the sandbox, click on the
<Select>
and wait for it to load.https://codesandbox.io/s/vqvm1704q3
Your Environment
The text was updated successfully, but these errors were encountered: