Skip to content

Remove explicit box-sizing for input[type="search"] #496

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

Closed
wants to merge 2 commits into from
Closed

Remove explicit box-sizing for input[type="search"] #496

wants to merge 2 commits into from

Conversation

patrickhlauke
Copy link
Contributor

This normalizes the CSS to what the suggested default browser CSS should be, which is now implemented in IE11, Edge, Blink, WebKit and Gecko (for the latter, see https://www.w3.org/Bugs/Public/show_bug.cgi?id=28784)

References twbs/bootstrap#17379

Closes #471

*/

input[type="search"] {
-webkit-appearance: textfield; /* 1 */
box-sizing: content-box; /* 2 */
box-sizing: border-box; /* 2 */
Copy link

Choose a reason for hiding this comment

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

Why not remove the box-sizing declaration entirely and leave the line blank?

When you need it to be border-box, you likely set everything to border-box. And if anyone really should just need the being border-box, I think he is better on doing that in his own styles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

to normalize older versions of Firefox (and potentially other, less common browsers with non-standard defaults)? otherwise fine, could also just be removed altogether

Copy link
Owner

Choose a reason for hiding this comment

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

What about setting border-box on input? I noticed that input[type=number] seems to use content-box in Chrome. And this would also keep the specificity at 0,0,0,1 which is easy to override; I think the specificity of this rule is probably more annoying/problematic than the styles it applies.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

interestingly, despite what is said here https://html.spec.whatwg.org/multipage/rendering.html#form-controls it seems most browser do indeed set box-sizing:content-box in their browser styles. wonder if now forcing border-box sizing generally would end up breaking layouts for devs who relied on this. perhaps the least harmful path would indeed be to just drop the box-sizing rule altogether from input[type="search"], now that firefox has harmonised with other browsers ?

Copy link

Choose a reason for hiding this comment

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

perhaps the least harmful path would indeed be to just drop the box-sizing rule altogether from input[type="search"]

Agree.

What about setting border-box on input

Wouldn't that be a minor update rather than a patch? I'm afraid it would break peoples styles in some cases, doesn't it?

@patrickhlauke patrickhlauke changed the title Set box-sizing for input[type="search"] to border-box Remove explicit box-sizing for input[type="search"] Jan 6, 2016
@patrickhlauke
Copy link
Contributor Author

(btw happy to squash / rebase this PR ... just didn't want to jump the gun here in case the decision to just nuke the box-sizing wasn't final)

@patrickhlauke
Copy link
Contributor Author

Rebased this to just remove the box-sizing CSS and test, as discussed.

@patrickhlauke
Copy link
Contributor Author

/cc @necolas for review?

@necolas
Copy link
Owner

necolas commented Jan 21, 2016

Thanks. Applied as fe56763

@necolas necolas closed this Jan 21, 2016
cvrebert added a commit to twbs/bootstrap that referenced this pull request Jan 21, 2016
cvrebert added a commit to twbs/bootstrap that referenced this pull request Jan 21, 2016
Revert "fixes #17379: override search input box-sizing to match our global overrides"

This reverts commits 468a9d9 & 57998dc.

necolas/normalize.css#496 has made this override unnecessary.

[skip sauce]
@patrickhlauke patrickhlauke deleted the input-search-border-box branch January 22, 2016 10:14
Totktonada added a commit to whatifrussian/website that referenced this pull request Oct 7, 2016
---- Summary ----

A text inside the issue form's textarea and the submit button should now
be gray when the element is disabled. Any other style changes are
unintentional and can be considered as bugs (if it looks worse then
before).

Despite some breaks are possible, my thought is that going forward with
actual normalize.css is good choice and any bugs will visible and
therefore will be fixed sooner or latter.

---- Details ----

The original intention was to get fix for color of disabled form's
elements from [1]. It removes `color: inherit` from rules for forms
elements, which affected enabled and disabled elements both. The
intention was to fix the issue form when some elements are disabled.

During testing of the new normalize.css I found that textarea is colored
black despite other text has some other default color (#2e3436). It's
maybe due to my browser has non-so-standard configuration (Firefox 47.0
with dark devedition theme enabled), but anyway it's possible user's
configuration. I added back `color: inherit` for issue form rules and
add `color: graytext` for disabled elements.

New normalize.css also replaces `font: inherit` for form elements with
choosen default (sans-serif, 100%, 1.15 line-height), see [2]. It
doesn't work well when the default font was replaced with some other.
So I added this rule back for the issue form.

There is one more found difference. Search form at the top of the page
cut from right side with the new normalize.css. It's due to reverting
the box-sizing rule, see [3]. I added back `box-sizing: content-box` to
the rules for search input element.

[1]: necolas/normalize.css#502
[2]: necolas/normalize.css#607
[3]: necolas/normalize.css#496
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants