-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Hide validation icons from multiple selects #33598
Hide validation icons from multiple selects #33598
Conversation
Hi @tkrotoff this mimics BS4 approach, any chance to take a look at this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about:
select { <=====
@include form-validation-state-selector($state) {
&[multiple],
&[size]:not([size="1"]) {
padding-right: $form-select-padding-x;
background-image: none;
}
}
}
instead of
.form-select {
@include form-validation-state-selector($state) {
&[multiple],
&[size]:not([size="1"]) {
padding-right: $form-select-padding-x;
background-image: none;
}
}
}
This way it fixes also your comment: #33411 (comment)
There should be no difference in moving from |
I see several solutions depending on whether #33411 (comment) should be treated or not (is Bootstrap 5 supposed to work if the user writes select.form-control instead of .form-select?) Modify scss/mixins/_forms.scss
Modifying _forms.scss has the drawback to have almost the same code inside _form-select.scss but the code can be factorized (something like remove-select-multiple-background-image()) Modify scss/forms/_form-select.scss
if we modify _form-select.scss this way, we don't need the same code inside _forms.scss The problem with !important is that the user cannot set its own background-image to the select/.form-select. I'm not a affiliated with Bootstrap, just my 2€ |
given @ffoodd's comment at #33411 (comment), would it be a better solution to only apply style to @if $enable-validation-icons {
&:not([multiple]),
&[size="1"] {
padding-right: $form-select-feedback-icon-padding-end;
background-image: escape-svg($form-select-indicator), escape-svg($icon);
background-position: $form-select-bg-position, $form-select-feedback-icon-position;
background-size: $form-select-bg-size, $form-select-feedback-icon-size;
}
} |
@tagliala I've tested This works (but does not solve #33411 (comment)): .form-select {
...
@if $enable-validation-icons {
&:not([multiple]):not([size]),
&[size="1"] {
padding-right: $form-select-feedback-icon-padding-end;
background-image: escape-svg($form-select-indicator), escape-svg($icon);
background-position: $form-select-bg-position, $form-select-feedback-icon-position;
background-size: $form-select-bg-size, $form-select-feedback-icon-size;
}
}
...
} Code to test: <h2>select form-control (supported by Bootstrap 4, should it be supported by Bootstrap 5?)</h2>
<div class="mb-3">
<select class="form-control is-valid">
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
<option>5</option>
</select>
</div>
<h2>form-select</h2>
<div class="mb-3">
<select class="form-select is-valid">
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
<option>5</option>
</select>
</div>
<h2>form-select multiple</h2>
<div class="mb-3">
<select class="form-select is-valid" multiple>
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
<option>5</option>
</select>
</div>
<h2>form-select size="1"</h2>
<div class="mb-3">
<select class="form-select is-valid" size="1">
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
<option>5</option>
</select>
</div>
<h2>form-select size="2"</h2>
<div class="mb-3">
<select class="form-select is-valid" size="2">
<option>1</option>
<option>2</option>
<option>3</option>
<option>4</option>
<option>5</option>
</select>
</div> Screenshot with untouched v5 beta3: btw you will notice that Bootstrap does not respect size="2" (it displays a bit more than 2 lines), screenshot with regular select size="2": |
Thanks, of course I was missing something btw, <form>
<code>form-select</code>
<div class="mb-3">
<select class="form-select is-valid">
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
<code>form-select size="1"</code>
<div class="mb-3">
<select class="form-select is-valid" size="1">
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
<code>form-select size="2"</code>
<div class="mb-3">
<select class="form-select is-valid" size="2">
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
<code>form-select multiple</code>
<div class="mb-3">
<select class="form-select is-valid" multiple>
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
<code>form-select multiple size="1"</code>
<div class="mb-3">
<select class="form-select is-valid" multiple size="1">
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
<code>form-select multiple size="2"</code>
<div class="mb-3">
<select class="form-select is-valid" multiple size="2">
<option>1</option>
<option>2</option>
<option>3</option>
</select>
</div>
</form> The correct inversion should be &:not([multiple]):not([size]),
&:not([multiple])[size="1"] { Bootstrap 4 vs Bootstrap 5 |
26a26c2
to
88f5834
Compare
Implementation provided in twbs#33411 does not take into account that some Operating Systems may display a vertical scrollbar in the multiple select field This implementation will hide the validation icons from multiple select fields, just like Bootstrap 4 does. Fix: twbs#33591
88f5834
to
491095a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, appreciate the effort in this PR! The selector specificity isn't my favorite, but it solves the problem well. I think we can ship this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, thanks for the discussions and researches!
Implementation provided in #33411 does not take into account that some
Operating Systems may display a vertical scrollbar in the multiple
select field
This implementation will hide the validation icons from multiple select
fields, just like Bootstrap 4 does.
Fix: #33591
Demo fiddles
BS4: https://jsfiddle.net/tagliala/y1t4cuqg/show
BS5: https://jsfiddle.net/tagliala/s45v1hLy/show
Before (Win 10 / Chrome 89)
Main
5.0.0.beta3
After (Win 10 / Chrome 89)
Bootstrap 4 vs Bootstrap 5