-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update input width in _chips.scss sass component #6567
base: master
Are you sure you want to change the base?
Conversation
This is a minor commit which updates the width of input field from fixed 120px to fit-content with same !important weight. Also, the optional min-width and max-width are set to limit its size between 100px and 200px. With this change, the input placeholder (if limited to under 200px) will not get cropped, and it will also have a better fit into the input field.
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.
Attention, this will not work on IE11. We should keep the old rule and add the new rule after it. So browsers who support it will use the second width rule.
https://caniuse.com/#feat=mdn-css_properties_width_fit-content
Yes @DanielRuf sir! Thank you for mentioning! It did actually slip out of my mind when I had made this pull request. I think even the default 'auto' value for width property might not work in the desired way for IE. Now, I guess it's time to update this PR. |
Regarding the new or old width setting? !important is bad in general. |
Actually, it was about the old width setting, as there was !important used with Still, as it was used in the previous code, I didn't remove it for now. Please let me know if in case it has to be discarded. |
This commit updates input width in _chips.scss sass component from fixed 120px value to fit-content for browsers (wherever supported), so that the input width takes up just the required amount of width, and having it between `min-width:100px` and `max-width:200px` constraints ensures that the input field will always be within just the required width, and will be now be lesser likely to crop placeholders.
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.
LGTM
This is a minor but suitable change which updates the width of input field (for adding new chips) from fixed 120px to fit-content, with same !important weight. Also, the min-width and max-width properties are set to limit its size between 100px and 200px. With this change, the input placeholder (if limited to under 200px) will not get cropped, and it will also have a better fit into the input field, thereby offering it more flexibility to adapt the content.
Proposed changes
Basically, this change fixes the issue that I had encountered recently with the placeholder or text of the input before pressing enter to convert it to a chip. Please have a look at how this small change affects the chips input (Screenshot from Google Chrome):
Screenshots (if appropriate) or codepen:
Types of changes
Checklist: