Skip to content
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] Breaks full width of TextField inside flex containers #20145

Closed
2 tasks done
levrik opened this issue Mar 16, 2020 · 7 comments · Fixed by #20538
Closed
2 tasks done

[AutoComplete] Breaks full width of TextField inside flex containers #20145

levrik opened this issue Mar 16, 2020 · 7 comments · Fixed by #20538
Labels
component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request

Comments

@levrik
Copy link

levrik commented Mar 16, 2020

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

As you can see in the example linked below, setting fullWidth on an TextField supplied to Autocomplete doesn't take the full width anymore as soon as the Autocomplete is wrapped inside a flex container.

Expected Behavior 🤔

That it takes the full width inside a flex container like a raw TextField does.

Steps to Reproduce 🕹

See https://codesandbox.io/s/trusting-bassi-7dtc1

Context 🔦

Your Environment 🌎

Tech Version
Material-UI v4.9.5
Material-UI Lab v4.0.0-alpha.95
React 16.13.0
@Cristy94
Copy link

The problem is that Autocomplete wraps the TextField in an extra <div>. You now have to add width: 100% or flex-grow: 1 to the Autocomplete component.

@levrik
Copy link
Author

levrik commented Mar 17, 2020

@Cristy94 I'm completely aware of this. But it feels a bit weird that I have to manually add styles to something. I also wonder why Autocomplete needs this div wrapper.

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Mar 18, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 19, 2020

I also wonder why Autocomplete needs this div wrapper.

@levrik It's required per https://www.w3.org/TR/wai-aria-practices/#combobox. But they are working on removing the container: https://github.com/w3c/aria/wiki/Resolving-ARIA-1.1-Combobox-Issues#aria-11-problems. I'm not sure it's stable enough yet. @eps1lon What do you think, should we follow this draft or delay things? w3c/aria#1051

That it takes the full width inside a flex container like a raw TextField does.

This is an interesting concern, I have been wondering about this problem lately: #19805.
Assuming we want to delay the change of ARIA from 1.1 to 1.2, I would suggest the following: We add a fullWidth prop at the Autocomplete level:

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
index 0c3181850..c65ba60be 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.js
@@ -20,6 +20,10 @@ export const styles = theme => ({
       visibility: 'visible',
     },
   },
+  /* Styles applied to the root element if `fullWidth={true}`. */
+  fullWidth: {
+    width: '100%',
+  },
   /* Pseudo-class applied to the root element if focused. */
   focused: {},
   /* Styles applied to the tag elements, e.g. the chips. */
@@ -376,6 +380,7 @@ const Autocomplete = React.forwardRef(function Autocomplete(props, ref) {
           classes.root,
           {
             [classes.focused]: focused,
+            [classes.fullWidth]: fullWidth,
             [classes.hasClearIcon]: hasClearIcon,
             [classes.hasPopupIcon]: hasPopupIcon,
           },

@levrik
Copy link
Author

levrik commented Mar 19, 2020

Adding fullWidth to the Autocomplete component sounds like a good idea as long as that wrapper div has to be kept.

@eps1lon
Copy link
Member

eps1lon commented Mar 19, 2020

I wouldn't touch anything combobox related right now without actually verifying it with assistive technology. If I remember correctly then ARIA 1.2 and 1.3 will have an updated version of the combobox pattern.

@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request labels Mar 19, 2020
@igorbrasileiro
Copy link
Contributor

First of all, I'm new here. I hope can help the material-ui community

The problem is that Autocomplete wraps the TextField in an extra <div>. You now have to add width: 100% or flex-grow: 1 to the Autocomplete component.

@Cristy94 and @oliviertassinari I tried to remove this extra div and the behavior still the same seemingly, I did a few tests.

Someone who has a good background about Material-UI, why need this extra div, have something special that I'm not seeing?

@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 21, 2020

Someone who has a good background about Material-UI, why need this extra div, have something special that I'm not seeing?

@igorbrasileiro It's required per https://www.w3.org/TR/wai-aria-practices/#combobox.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants