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

set min-height for form-control textarea #29124

Merged
merged 4 commits into from
Aug 18, 2019

Conversation

m5o
Copy link
Contributor

@m5o m5o commented Jul 24, 2019

  • set min-height for .form-control textarea
    • prevent user from shrink textarea
    • set minimum height equals default input height
before after
Screenshot 2019-07-24 at 16 40 50 Screenshot 2019-07-24 at 16 41 13
Screenshot 2019-07-24 at 17 36 03 Screenshot 2019-07-24 at 17 35 18

📷 Chrome 75 / Mac OS X 10.14.6

@m5o m5o requested a review from a team as a code owner July 24, 2019 15:17
@ysds
Copy link
Member

ysds commented Jul 24, 2019

To fix also #28901, is it better add min-height to .form-control and remove the specific heights?

@MartijnCuppens
Copy link
Member

@ysds, that would reintroduce #18842, unless #28917 is merged.

@ysds
Copy link
Member

ysds commented Jul 29, 2019

@m5o Would you add the min-height to .form-control and .form-select directory and remove the specific height from those? Also need to modify the sizing options and the combinations with .input-groups.

@m5o m5o force-pushed the forms-textarea-min-height branch from 5078a21 to ba8fa47 Compare July 30, 2019 07:43
@m5o
Copy link
Contributor Author

m5o commented Jul 30, 2019

@ysds done, hopefully as you requested.

🤔 I don't know if, for consistency, it would make sense to set min-height also to .form-file

Copy link
Member

@ysds ysds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m5o nice, LGTM 👍

@ysds
Copy link
Member

ysds commented Aug 5, 2019

.form-file use position: absolute, so would have to use height.

Copy link
Member

@MartijnCuppens MartijnCuppens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Firefox add 2px to its select because of this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1560824, so we should set height on selects unfortunately

@ysds
Copy link
Member

ysds commented Aug 6, 2019

umm, confirmed

image

@mdo
Copy link
Member

mdo commented Aug 15, 2019

@MartijnCuppens Can you re-review, please?

@mdo mdo added this to Inbox in v4.4.0 via automation Aug 15, 2019
@XhmikosR XhmikosR added this to Inbox in v5 via automation Aug 16, 2019
@XhmikosR
Copy link
Member

Are we sure about backporting this BTW?

@m5o m5o requested a review from a team as a code owner August 17, 2019 11:48
@MartijnCuppens
Copy link
Member

#18842 still seemed to be an issue. I've pushed a working fix (219e1c68ce6405778bcaf865841cc89f3d80b7c6) and also committed a fix for the input groups.

v5 automation moved this from Inbox to Approved Aug 17, 2019
m5o and others added 3 commits August 18, 2019 16:21
* set `min-height` for `.form-control` textarea
  * prevent user from shrink textarea to minimum height
@MartijnCuppens MartijnCuppens merged commit 9452120 into twbs:master Aug 18, 2019
v5 automation moved this from Approved to Shipped Aug 18, 2019
@XhmikosR
Copy link
Member

@MartijnCuppens: this doesn't apply clean on v4-dev-xmr. Can you backport it manually please?

@MartijnCuppens
Copy link
Member

I'm afraid we can't backport this easily since .form-controls can be selects too in v4. To make this work we'll need to rewrite quite a lot and this might introduce breaking changes.

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

Successfully merging this pull request may close these issues.

None yet

5 participants