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

Replace select2 with <select class="custom-select"> #2034

Merged
merged 24 commits into from
Jun 28, 2017

Conversation

jhawthorn
Copy link
Contributor

@jhawthorn jhawthorn commented Jun 21, 2017

This replaces the select2 component with a plain <select> (with bootstrap's custom-select styling) throughout the admin.

@jhawthorn jhawthorn added changelog:solidus_backend Changes to the solidus_backend gem WIP labels Jun 21, 2017
@jhawthorn jhawthorn changed the title [WIP] Replace select2 with custom-select Replace select2 with <select class="custom-select"> Jun 22, 2017
@jhawthorn jhawthorn removed the WIP label Jun 22, 2017
@mtomov
Copy link
Contributor

mtomov commented Jun 22, 2017

Hm.. Ain't it a more valid approach to style all select with your custom styles, but maybe add a no-custom-style or so class for those occasions when you want to opt out of it. That should make it easier for developers as they won't have to learn about this new class unless they have a very very special case to "opt out". Would certainly help extensions as well.

Edit: Sorry misread the PR, ignore the above - so there's actually no additional styling that comes with this new class custom-select .. then why do we need it at all is my new question?

@jhawthorn
Copy link
Contributor Author

custom-select has styling from bootstrap. It is somewhat heavy handed, so we probably don't want to @extend it onto every select element.

@gmacdougall gmacdougall merged commit 55a72e7 into solidusio:master Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:solidus_backend Changes to the solidus_backend gem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants