lcreid / devise-bootstrap-form Goto Github PK

The wrapper around all the forms may be a good idea for one use case, but it's not really generalizable. For example, if you want the login to appear both stand-alone, and in the middle of the landing page, you might want to use the same views but rendered in different contexts.

Also, we might not want the same context for all the views. The password reset, resend confirmation, etc. views may have a different look than the simple sign-in/sign-up views.

It might be better to just document ways to address how to make the views consistent without building it into this gem.

The _error_messages.html.erb has translations in it, even for the un-internationalized version of the views.

The shared links don't look that great as links, and the spacing of them sucks if you convert them to buttons. The generated templates could be more friendly to modifying the styles and spacing.

It would be more flexible if the wrapper for the devise-bootstrap-forms views were a partial around each view:

• In one place you can custom format the views using Bootstrap classes, using Bootstrap from CDN, and without having to run SASS.
• It's two lines to strip from each view, instead of six, if you don't want any of devise-bootstrap-form's formatting.
• It's easier to do a completely custom format around the Devise views.

• Confirm the partial approach will work.
• Write the documentation.
• Make the changes.
• Release.

We use the default behaviour of bootstrap_form to report errors, meaning that the errors are displayed "in line", in an area below the field that corresponds to the error. The views still have the call to devise_error_messages! in them, so errors are reported twice.

If we remove the call to devise_error_messages!, we still have to account for errors that might not correspond to a field. I believe the Rails documentation suggests that they should get a key of :base in the error collection (need to confirm this).

One solution would be to override devise_error_messages!, but I'm not a fan of magically overriding expected behaviour.

We could create a devise_bootstrap_form_error_messages! that output error messages for :base formatting consistently with the rest of the look and feel of the form.

Bonus marks if this method could detect if the form had the in-line option set, and produce the full set of messages if not. That would imply that the error message method would take the form builder as an argument.