Replace select2 styling #1797
Replace select2 styling #1797
Conversation
jhawthorn
force-pushed
the
select2_styling
branch
from
2c9472f to
c8c8d2a
Compare
|
Updated this to use the same dropdown arrow as bootstrap's |
|
👏 |
jhawthorn
force-pushed
the
select2_styling
branch
from
c8c8d2a to
2206c3d
Compare
|
This is out of WIP For icons, I only brought back the "close" icon from font-awesome. The dropdown icon is provided by bootstrap, and I chose to forgo the search icon, which I don't think was useful. Colours come from bootstrap variables, which will be slightly off until #1809 is merged. |
jhawthorn
force-pushed
the
select2_styling
branch
from
2206c3d to
33f3881
Compare
Fixes for the changes I made in the PR review solidusio#1797
There was a problem hiding this comment.
First of all, thanks @jhawthorn for the work on this. Adjusting the Select2 styles is way overdue 🥇
Achtung! This is a very long review, just because Frontend ¯\_(ツ)_/¯
My comments are mostly about pushing pixels, avoiding magic numbers and embracing Bootstrap as much as we can.
I pushed my changes to tvdeyen@e11d329 so you can easily grab them if you agree with them.
Again: 🎉 Thanks! 🍻
| border: $input-btn-border-width solid $input-border-color; | ||
| @include border-radius($input-border-radius); | ||
| background: $input-bg; | ||
| height: $input-height; |
There was a problem hiding this comment.
Somehow the height isn't correct when using this variable (it's two pixel off). I guess somehow the calculation of $input-height is wrong. It seems the correct formula, though. 🤔
Taking the$input-btn-border-width into account seems to fix this issue.
.select2-choice, .select2-choices {
...
height: calc(#{$input-height} + #{2*$input-btn-border-width});*) calc only works with variables in Sass if you interpolate them. We need to use calc here, because of mismatching units (rem and px). But as calc is very widely supported, I think this is a good compromise.
Before
After
| &.select2-dropdown-open .select2-choice div b { | ||
| @extend .fa-caret-up | ||
| >.select2-chosen { | ||
| color: $input-color; |
There was a problem hiding this comment.
Unfortunately with the fix for the height above it's even more obvious that the height of the selected value does not match its parent container.
It think this is a good fix for that:
>.select2-chosen {
line-height: $input-height;
| content: "\f0d7"; | ||
| } | ||
| } | ||
| line-height: 17px; /* Fill height of input */ |
There was a problem hiding this comment.
Line heights should always be unit-less. Unfortunately we can't use the $line-height variable (tried that, doesn't fit into the container very well).
A value of 1.35 does it for me in Firefox and Chrome, though.
In order to make this work well with the search input field and we get a consistent look I came up with this solution:
.select2-container-multi .select2-choices {
.select2-search-choice, .select2-search-field {
line-height: 1.35; /* Fill height of input */
}
.select2-search-choice {
background: $label-default-bg;
border: 0;
margin: 2px 0 3px 5px; // adjust the margin to respect the adjusted line height
}
}Before
After
| .select2-results { | ||
| .select2-no-results, .select2-searching { | ||
| /* Remove light grey background */ | ||
| background: transparent; |
There was a problem hiding this comment.
Can we also adjust the height a little bit?
.select2-no-results, .select2-searching {
...
line-height: 2.5;Before
After
There was a problem hiding this comment.
While we at it, we should adjust the select highlight colours.
$dropdown-link-hover-color: #fff !default;
$dropdown-link-hover-bg: $color-3 !default;.select2-results {
...
.select2-highlighted {
background-color: $dropdown-link-hover-bg;
// Ensure all remote results have correct colors on hover
* { color: $dropdown-link-hover-color }
}
}Before
After
| background-image: none; | ||
| background: transparent; | ||
| .select2-container-multi .select2-choices .select2-search-choice { | ||
| background: #e4e4e4; |
There was a problem hiding this comment.
We should use a Bootstrap variable for this. $label-default-bg seems to be a great fit. Not sure about this, though, but we should not hardcode a value here.
$label-default-bg: $color-7 !default;
Previously, we styled select2 elements to be very blue (under default colours). This didn't match the visual language we used for the rest of our inputs, which have a plain white background. The select2 inputs had a style more similar to buttons, which hold a different purpose. This commit replaces the existing select2 styling with styling meant to match the look of a standard select element with bootstrap's styling. This looks very similar to select2 v4's styling, and we had previously intended to switch to select2 v4's default styling after upgrading, but it no longer looks like we will be upgrading.
Fixes for the changes I made in the PR review solidusio#1797
jhawthorn
force-pushed
the
select2_styling
branch
from
33f3881 to
001b702
Compare
A year ago I submitted a similar PR (#861) which removed the select2 styling. At the time we tabled it because we intended to upgrade to select2 4.0, from which we preferred the styles. I no longer think we will be performing that upgrade (see #763), so giving this another shot.
The existing styles we had don't match the visual language we used for the rest of our inputs (flat, thin border, white background) and instead more closely matched buttons (solid blue background) which have a different purpose.
This replaces our existing custom select2 styles with much simpler styles intended to better match our other input elements. This also matches the look of bootstrap-styled select elements, which would allow us to use those in the future.
After
Before
Todo