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

Form control heights #26820

Merged
merged 2 commits into from Jul 15, 2018
Merged

Form control heights #26820

merged 2 commits into from Jul 15, 2018

Conversation

mdo
Copy link
Member

@mdo mdo commented Jul 8, 2018

Somehow I missed that #18842 and #18843 were still open. In my testing of the macOS Mojave beta, Safari has fixed their issue with the sizing, but Chrome still has the issue. It also apparently is a bug marked as won't fix. Thus, the only sensible solution to consistent .form-control heights across all supported input types in a single browser is to specify heights. I've been resisting this thinking it'd be resolved by browsers, but that's no longer the case.

Here's what's changed:

  • Applies the already present $input-height to .form-control.
  • Consolidates the <select> size and multiple overrides into just the .form-control base class instead of -sm/-lg modifiers.
  • Removes the Sass @extends from input groups since it picks up too many selectors, namely our <select> overrides.

Seen this demo of the changed CSS and inputs/selects.

Related test JS Bin example.

Closes #18843, fixes #18842, fixes #25923, fixes #26648, and fixes #26209.

@mdo mdo force-pushed the input-height branch 2 times, most recently from 7d0931b to c7a5d43 Compare July 8, 2018 05:12
@mdo mdo added this to Inbox in v4.1.2 via automation Jul 8, 2018
@mdo mdo added this to Inbox in v4.1.3 via automation Jul 8, 2018
@mdo mdo removed this from Inbox in v4.1.2 Jul 8, 2018
@mdo mdo moved this from Inbox to Ready to merge in v4.1.3 Jul 9, 2018
@danijelGombac
Copy link

@mdo

There should be also textarea height auto when rows attribute is applied.

- Use the already present -height variables on .form-control
- Consolidate the select size and multiple overrides into the .form-control base class instead of sm/lg modifiers
- Remove the Sass extends from input groups since it picks up too many selectors
@mdo mdo merged commit f426a67 into v4-dev Jul 15, 2018
v4.1.3 automation moved this from Ready to merge to Shipped Jul 15, 2018
@mdo mdo deleted the input-height branch July 15, 2018 21:00
@mdo mdo mentioned this pull request Jul 15, 2018
@hrvojegolcic
Copy link

I think this might need to be reopened?

After the fix now the .input-group is now broken if you have change the height of an input group .input-group:

Comparism of v4.0 and the current v4.1, when you set height: 5em to .input-group:

image

@danijelGombac
Copy link

danijelGombac commented Aug 1, 2018

@hrvojegolcic

You can add h-auto to form-control.

@hrvojegolcic
Copy link

@danijelGombac Thanks, I did figure the workaround but the thing is still broken now in comparison to v4.0, because .input-group-text is taking advantage of flexbox property align-items:stretch and .form-control has a fixed height. It's inconsistent. If you ask me, either both have to have hardcoded widths or none of them.

kennyadsl added a commit to nebulab/solidus that referenced this pull request Nov 9, 2018
In latest Twitter Bootstrap versions form-control height is
no more auto and this change is needed to make our search
bar work properly, see: twbs/bootstrap#26820
johnmcdowall added a commit to kiso-io/rrt-gem that referenced this pull request May 11, 2019
@nfxpnk
Copy link

nfxpnk commented Dec 13, 2019

1px padding on Windows, latest Chrome with height property on v4.4.1

bootstrap

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
v4.1.3
  
Shipped
Development

Successfully merging this pull request may close these issues.

None yet

4 participants