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

Simpler input groups #31666

Closed
wants to merge 10 commits into from
Closed

Simpler input groups #31666

wants to merge 10 commits into from

Conversation

mdo
Copy link
Member

@mdo mdo commented Sep 16, 2020

This PR rewrites input groups, replacing the .input-group class with two new classes: .input-group-start and .input-group-end. Alongside that, this drops all support for multiple addons, dropdowns, button groups, and multiple inputs. Together, has the advantage of fixing our longstanding rounded corner issue with form validation.

All those variations in the component make for an immense amount of difficulty in supporting it long term, and missing important bugs like the rounded corners with form validation. By simplifying things like this with two specific class names, we gain a ton of control.

If you find yourself needing a layout that uses all these now dropped variations, a standard "inline" form layout is most likely the best option (more control over spacing, alignment, and responsive behaviors).

Still more to be done, but I'm liking this simplified direction.

This fixes #25110 and closes #30170. With the changes from #31677, this also fixes #28414.

Thoughts @twbs/css-review?

@mdo mdo added this to Inbox in v5.0.0-alpha3 via automation Sep 17, 2020
@mdo mdo marked this pull request as ready for review September 17, 2020 22:15
@mdo mdo requested a review from a team as a code owner September 17, 2020 22:15
@mdo mdo moved this from Inbox to Review in v5.0.0-alpha3 Sep 18, 2020
@vanillajonathan
Copy link
Contributor

Consider a .input-group-between class. #25120

@mdo
Copy link
Member Author

mdo commented Sep 30, 2020

Consider a .input-group-between class. #25120

Given we're no longer supporting multiple inputs, this feels potentially problematic. I'll take a look and see what can be done, but can't promise it'll happen for sure.

@opensums-paul
Copy link

"this drops all support for multiple addons, dropdowns, button groups, and multiple inputs"

That's a big loss - no more | - | 10 | + | value adjusters, | << | < | > | >> | pagination controls etc. etc.

@MartijnCuppens
Copy link
Member

Not a fan of this. With this change we won't be able to support multiple input text. I got a feeling, this will introduce more issues than we have now.

@mdo
Copy link
Member Author

mdo commented Oct 21, 2020

We can do a .input-group-around and set classes for addons on both, but having a single class that hits everything is what led us to the issues with rounded corners and form validation.

@opensums-paul Having them on both sides should be doable with latest commit, and without side effects of other components, form validation, etc. Pagination controls, I'd suggest sticking to the pagination component. I know some folks want the jump to a page with an input in there, so maybe we figure out how to do that with our pagination.

@mdo
Copy link
Member Author

mdo commented Oct 21, 2020

With this change we won't be able to support multiple input text. I got a feeling, this will introduce more issues than we have now.

Other than aesthetics, it hit me while working on this that multiple inputs aren't all that necessary. A super wide set of buttons, addons, and inputs doesn't feel all that useful IMO when it could be done with default buttons and form controls already.

What kind of issues are you thinking we'd run into? With a simpler approach and fewer supported permutations, my expectation is that we'll have far fewer issues and greater interoperability with other components, styles, themes, etc.

@vanillajonathan
Copy link
Contributor

vanillajonathan commented Oct 26, 2020

Having a input group between two inputs seem to work fine in Bootstrap v5.0.0-alpha2.

<div class="input-group">
  <input type="text" class="form-control" placeholder="Username">
  <span class="input-group-text">@</span>
  <input type="text" class="form-control" placeholder="Server">
</div>

I would like to see this added as an example in the documentation.

@mdo
Copy link
Member Author

mdo commented Oct 26, 2020

Closing for #31953.

@mdo mdo closed this Oct 26, 2020
@mdo mdo removed this from Review in v5.0.0-alpha3 Oct 26, 2020
@MartijnCuppens MartijnCuppens deleted the simpler-input-groups branch October 26, 2020 22:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants