-
-
Notifications
You must be signed in to change notification settings - Fork 32.4k
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
Autocomplete with Popper breaks positioning #13179
Comments
also having this issue |
That also happens when the number of items in autocomplete (using downshift + popper approach and top positioning) reduces from the max entries. |
Basic issue is that the offset is only calculated on open and window resize. Using a resize observer would be possible but needs a 2Kb polyfill for IE 11 (we could just use it if it's available). Another solution is to measure the element in |
@eps1lon I get the measure part, but how can we force to change the style when the size has changed? |
That was more aimed at a possible fix to our codebase. A workaround for now is to make use of the |
Ok, thanks anyway. I couldn't make autocomplete (with downshift) work with Popover, so I probably will hold material-ui migration until this is resolved. |
@eps1lon, Your idea is OK, but there is a problem with downshift and popover that when |
@Filson14 This issue is not about the popover blocking input. Could you open an issue if that is the case for you? It looks like Popper is currently missing an API to update the position similar to As a temporary workaround you can force a remount of the Popper when you would call Popover#updatePosition by changing the |
@eps1lon I agree, this sounds like an excellent idea! @Filson14 What do you think of: --- a/docs/src/pages/demos/autocomplete/IntegrationDownshift.js
+++ b/docs/src/pages/demos/autocomplete/IntegrationDownshift.js
@@ -244,6 +244,15 @@ const styles = theme => ({
});
let popperNode;
+let updatePosition;
+
+function UpdatePosition() {
+ if (updatePosition) {
+ updatePosition();
+ }
+
+ return null;
+}
function IntegrationDownshift(props) {
const { classes } = props;
@@ -310,7 +319,13 @@ function IntegrationDownshift(props) {
popperNode = node;
},
})}
- <Popper open={isOpen} anchorEl={popperNode}>
+ <Popper
+ action={actions => {
+ updatePosition = actions.updatePosition;
+ }}
+ open={isOpen}
+ anchorEl={popperNode}
+ >
<div {...(isOpen ? getMenuProps({}, { suppressRefError: true }) : {})}>
<Paper
square
@@ -328,6 +343,7 @@ function IntegrationDownshift(props) {
</Paper>
</div>
</Popper>
+ <UpdatePosition />
</div>
)}
</Downshift>
diff --git a/packages/material-ui/src/Popper/Popper.js b/packages/material-ui/src/Popper/Popper.js
index fb53a9c42..f8f32359e 100644
--- a/packages/material-ui/src/Popper/Popper.js
+++ b/packages/material-ui/src/Popper/Popper.js
@@ -40,6 +40,18 @@ class Popper extends React.Component {
};
}
+ componentDidMount() {
+ if (this.props.action) {
+ this.props.action({
+ updatePosition: () => {
+ if (this.popper) {
+ this.popper.scheduleUpdate();
+ }
+ },
+ });
+ }
+ }
+
componentDidUpdate(prevProps) {
if (prevProps.open !== this.props.open && !this.props.open && !this.props.transition) {
// Otherwise handleExited will call this.
@@ -139,6 +151,7 @@ class Popper extends React.Component {
render() {
const {
+ action,
anchorEl,
children,
container,
@@ -186,6 +199,15 @@ class Popper extends React.Component {
}
Popper.propTypes = {
+ /**
+ * This is callback property. It's called by the component on mount.
+ * This is useful when you want to trigger an action programmatically.
+ * It currently only supports updatePosition() action.
+ *
+ * @param {object} actions This object contains all possible actions
+ * that can be triggered programmatically.
+ */
+ action: PropTypes.func,
/**
* This is the DOM element, or a function that returns the DOM element,
* that may be used to set the position of the popover. It's a quick POC, we would be happy to review a pull request :). |
Nice idea with the key! Thanks. |
@Filson14 It is intended that a opening a popover moves focus to it. It's a modal that has autofocus and focus trap built into it. I don't think that's the right tool to use here.
I would recommend you stick with the demo since it should already cover most a11y concerns. |
Like I said, I get it 😉 In my case, using Popper with key work OK. Thanks again. |
An update on the solution I have proposed in #13179 (comment).
|
@oliviertassinari is the fix shipped? I am using Version 4.0.0-alpha.36. But still seeing this issue. |
This issue covers a legacy version of the component. I'm sorry, it's off topic. |
I am using material-ui/lab version 4.0.0-alpha.36. And I am having this exact issue. Should I submit a new issue? |
Yes please, we need a reproduction too. |
I was having this exact same problem and after 2 hours of bashing my head against the wall since the demo on mui site doesn't have this problem I decided to upgrade both core and lab to latest versions and it started working. My versions when it wasn't working: If anyone else is having this issue just upgrade to latest. |
@oliviertassinari any help, please? |
if autocomplete is inside a container that contains a zoom property. popper position is always incorrect.
Any workaround to avoid this? |
@anmolmalik97 You can replace the |
@oliviertassinari thank you for such a quick response. I tried using it. But using div as a popperComponent pushes the other components away when it opens up. |
I would like for the position of the list to change if I filter based on specific value.
Right now if I filter the position stays the same and seems wrong, and if I scroll with my mouse then it correctly places itself above the input
Expected Behavior
Position of the Popper should be recalculated on input change
Current Behavior
Position doesn't get recalculated, only on scroll.
Steps to Reproduce
Context
Use autocomplete
Your Environment
Just the https://material-ui.com/demos website.
Browser: Chrome Version 69.0.3497.100 (Official Build) (64-bit)
The text was updated successfully, but these errors were encountered: