fix(multi-select): remove absolute positioning of filterable tag, update input spacing #4760
Conversation
ghost
requested review from
jendowns
changed the title
|
Deploy preview for the-carbon-components ready! Built with commit 2adc449 https://deploy-preview-4760--the-carbon-components.netlify.com |
|
Deploy preview for carbon-components-react ready! Built with commit 2adc449 https://deploy-preview-4760--carbon-components-react.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit 2adc449 |
|
Deploy preview for carbon-elements ready! Built with commit a08f4f3 |
|
Deploy preview for carbon-components-react ready! Built with commit a08f4f3 https://deploy-preview-4760--carbon-components-react.netlify.com |
|
Deploy preview for the-carbon-components ready! Built with commit a08f4f3 https://deploy-preview-4760--the-carbon-components.netlify.com |
|
@jendowns is there any way we could get the focus indicator to span the entire control with this approach? Just going by the screenshots it seems like this detail has changed so wanted to check 👍 |
|
Hey @joshblack let me know if this makes sense: The tag is a sibling of the input. Currently, the tag is absolutely positioned on top of the input. But the tag's content is dynamic / user-controlled. It's not really the same as a static icon with a consistent size, that you can reliably place on top of something with a lot of padding... so, absolutely positioning the tag inside the padding of the underlying input does not work with 10+ selected items. I don't think there's a way, with CSS alone, to make it so the padding of the input -- where the tag is absolutely positioned on top of -- expands to accommodate a growing character count in the tag. And since they are siblings + the input doesn't have a true So that's why I removed the absolute positioning. I don't think it's possible to keep this looking like the non-filterable |
|
@jendowns would you want to pair up to help catch me up on what you've run into and detailed above? Would love to work on this with ya if you have the time! |
|
Hey @joshblack I was just trying to suggest a way to fix something that is broken at the moment. 😅 The tag is absolutely positioned, yet is dynamic content... and it's locked up next to an input. It will continue to overlap the input as the dynamic content (i.e., character counter of tag content) changes. So I just wanted to suggest making it not absolutely positioned as a quick fix so the input isn't covered anymore. Preserving the focus outline would need a component refactor of some level. If you'd prefer that route instead, I can close out this PR. Let me know -- thank you! |
|
@jendowns totally fair! Sorry for the confusion. I think we would like to keep the focus outline on the whole control and definitely would be down to refactor if we need to in order to support the selected count growing 👍 |
|
I would think you'd need flexbox sizing to let the app specify the total width of the multiselect, and it would give the buttons however much space they need, and give the rest of the space to the |
Closes #4721
#4721 appears to be caused by an absolutely positioned tag. The tag sits on top of the filter input, so as the number inside the tag grows it will continue to encroach on the filter input's text.
This fix removes absolute positioning for that selected items tag.
A side effect of this is that when you select the filter input now, the focus outline will no longer extend / wrap everything --
Old (with 10+ selected items):
New (with 10+ selected items):
☝️ For the spacing above, I can definitely adjust the split between spacing inside the input and spacing between the input and the tag. Just let me know what you prefer!
But due to the structure of this component, this seems like the easiest fix. I imagine with a bigger refactor + involving some JS, you could possibly keep the tag on top of the input and just steadily adjust the input's left padding to accommodate a growing "selected items" number in the tag. That seems like a lot of overhead though 😅 What do you all think?
Changelog
Changed