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

Feature/boostrap classes for stripe errors #238

Merged
merged 4 commits into from Feb 10, 2016
Merged

Feature/boostrap classes for stripe errors #238

merged 4 commits into from Feb 10, 2016

Conversation

berkes
Copy link

@berkes berkes commented Oct 21, 2015

This adds some classes to the Stripe partial, which hook into the boostrap styles for rendering errors.

  • A class to render the common error message in an alert-box.
  • A class added to the parent of an erroring field, the way boostrap wants errors set on form-fields.

After the styles are added, the form looks something like:
schermafbeelding 2015-10-21 op 19 51 06

I failed to run the tests, somehow the Bundle was conflicting, possibly a conflict in rbenv. However, this is only JS, and as far as I can see, this is not tested anyway.

Additionally, we leave the original "error" class addition to the field
itself intact. So that peoples CSS won't break.
This will render it with the boostrap "alert" styles.
@berkes
Copy link
Author

berkes commented Oct 22, 2015

Turns out the CI has the same trouble with running the tests as I do.

@berkes
Copy link
Author

berkes commented Oct 26, 2015

I've rebased on #239 so that the CI can run the tests on this PR.

damianlegawiec added a commit that referenced this pull request Feb 10, 2016
…pe_errors

Feature/boostrap classes for stripe errors
@damianlegawiec damianlegawiec merged commit 659bcf5 into spree:master Feb 10, 2016
@berkes berkes deleted the feature/boostrap_classes_for_stripe_errors branch February 11, 2016 08:37
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.

None yet

2 participants