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

Make our form/button styles match bootstrap's #1809

Merged
merged 13 commits into from
Mar 31, 2017

Conversation

jhawthorn
Copy link
Contributor

This makes our forms (inputs and buttons) match those using bootstrap classes (.btn and .form-control). This should allow us to progressively change our current forms markup to bootstrap.

Similar to the changes in #1780, bootstrap variables are set to the legacy styles' values.

Buttons are styled as bootstrap buttons using @extend. @extend isn't an ideal tool to use, but I looked at the CSS products and it was okay here.

This necessitated one change: buttons hover color is now dark blue as it's hardcoded in bootstrap to be 10% darker than the non-hovered BG. I hope this is okay. The new style is nicer. It currently disagrees with the select2 colours, but that will be fixed by #1797

I tried using @extend for the form inputs, but the CSS produced was ridiculous. We also don't want some of the styles that would imply (like width: 100%). Instead I've just used the bootstrap variables here and pulled in their placeholder pseudo-element styling.

@jhawthorn jhawthorn added changelog:solidus_backend Changes to the solidus_backend gem changelog:solidus_api Changes to the solidus_api gem labels Mar 29, 2017
@jhawthorn jhawthorn requested a review from tvdeyen March 29, 2017 22:52
@jhawthorn jhawthorn changed the title Make our forms match bootstrap's Make our form/button styles match bootstrap's Mar 29, 2017
@jhawthorn jhawthorn added UI and removed changelog:solidus_api Changes to the solidus_api gem labels Mar 29, 2017
@jhawthorn jhawthorn mentioned this pull request Mar 30, 2017
2 tasks
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Awesome (have a small change regarding deprecated variables)

@@ -289,16 +289,16 @@ $input-padding-y: .375rem !default;
$input-bg: #fff !default;
$input-bg-disabled: $gray-lighter !default;

$input-color: $gray !default;
$input-border-color: #ccc !default;
$input-color: $color-txt-text !default;
Copy link
Member

Choose a reason for hiding this comment

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

We deprecated the $color-txt-text variable. Can we use $color-3 instead?

Copy link
Contributor Author

@jhawthorn jhawthorn Mar 30, 2017

Choose a reason for hiding this comment

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

We've deprecated it, which means if a user sets $color-txt-text, they should see a warning, but it should continue working. This line (and the few below) are necessary for it to continue working

Copy link
Member

Choose a reason for hiding this comment

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

Your are right. Ignore me

$input-color: $gray !default;
$input-border-color: #ccc !default;
$input-color: $color-txt-text !default;
$input-border-color: $color-txt-brd !default;
Copy link
Member

Choose a reason for hiding this comment

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

dito

$input-btn-border-width: $border-width !default; // For form controls and buttons
$input-box-shadow: inset 0 1px 1px rgba(0,0,0,.075) !default;

$input-border-radius: $border-radius !default;
$input-border-radius-lg: $border-radius-lg !default;
$input-border-radius-sm: $border-radius-sm !default;

$input-border-focus: #66afe9 !default;
$input-border-focus: $color-txt-hover-brd !default;
Copy link
Member

Choose a reason for hiding this comment

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

dito

@jhawthorn jhawthorn merged commit f491eec into solidusio:master Mar 31, 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.

2 participants