clearance
clearance copied to clipboard
No validation messages for sign up
When I clicked sign up when no email and password inputted, it doesn't have flash message compare like signing in. Is this normal? and I need to set my own?
Do you have a suggestion for what the flash should say? Unfortunately, I can't think of anything particularly useful that wouldn't then be an impediment for people with different sign up requirements.
I thin the proper way to handle this is to customize the sign up template to display errors as you would see fit. It's a tough thing for us to do for you because error styling is generally very app-by-app.
What do you think?
Hi Derek,
You are right. I did use Devise before trying out Clearance, and in Devise I think there is a default error message. For me I would like have a default error message where we could customize to our liking. It's not really important just a little thing that I observed.
On Fri, May 8, 2015 at 10:51 PM, Derek Prior [email protected] wrote:
Do you have a suggestion for what the flash should say? Unfortunately, I can't think of anything particularly useful that wouldn't then be an impediment for people with different sign up requirements.
I thin the proper way to handle this is to customize the sign up template to display errors as you would see fit. It's a tough thing for us to do for you because error styling is generally very app-by-app.
What do you think?
— Reply to this email directly or view it on GitHub https://github.com/thoughtbot/clearance/issues/554#issuecomment-100260451 .
I posted a similar issue today with #559. I agree that perhaps there should be a default error. Even the "Bad email or password." would be good. That being said, this feels more like a form validation issue. Adding form errors in the sign up form makes some sense to me. Thoughts?
I usually see people create a place for flash messages in Rails view layouts, as @derekprior mentioned.
Looks like we are setting the flash message in the sign in form like @iamarmanjon mentioned. (reference: https://github.com/thoughtbot/clearance/blob/master/app/controllers/clearance/sessions_controller.rb#L13) - I think it makes sense to do the same in the UsersController so that the error messages display in the same way during sign up.
One challenge with this is that we are using a single default failure message when a sign in fails. For a sign up form where a user record is being created, failure messages are usually determined by ActiveRecord validations. There are validations here: https://github.com/thoughtbot/clearance/blob/aefdc076fe92ed5168bbc1984dec66f8475d0654/lib/clearance/user.rb#L42 - but I would have to think a little bit about how to use them in the controller template because we'd have to make sure to include any additional validations on the class that is using clearance.
@jessieay would it make sense to include form errors within the sign up form?
Something like...
<% if object.errors.any? %>
<h3>Please fix the following errors:</h3>
<ul>
<% object.errors.full_messages.each do |msg| %>
<li>
<%= msg %>
</li>
<% end %>
</ul>
<% end %>
That should allow us to display validation errors directly in the form instead of displaying a general flash message. Thoughts?
This is my least favorite part of Clearance to make decisions on... It's hard to get shared UI right at all. I'd be perfectly fine making some of these changes if generating the views into your own app was required and this was just a default option. But it's not required -- you can run with internal views.
I like the idea of setting the flash and as @jessieay points out, we're already doing that elsewhere. But I'm just not sure what that message should actually say. I don't think I'd consider this a breaking change, though some could argue it is. Clearance already assumes you display and style flashes. If your layout doesn't show them, then you won't get them, if it does, it should be displayed acceptably.
Presenting errors as @DavidVII mentions makes it obvious how to handle the messaging, but its also something I would consider a breaking change. There's no expectation for your sign up form to have styling to handle that markup in a nice way. That markup is almost exactly what error_messages_for did in Rails 2.3, but it was removed because it pre-supposes too much about how end apps should display errors. Error display is seldom identical across apps, in my experience.
Good points, @derekprior. I definitely think this requires some more thought.
IMO, the user flow is off when using internal views or even default controllers. Clearance has user validations in place, yet we're not showing their errors by default. Not showing these error messages when a user signs up adds a hurdle to the user experience. I feel like presenting errors as I described above makes it clear why there was an interruption. Given, my idea may not be the best solution, but I definitely feel like we need something. Thoughts?
I'm leaning towards making this a Clearance 2.0 change, adding errors to the view, and possibly requiring that views be dumped to your app and not served internally.
+1 on requiring the views
Hi all! For some reason when I was using clearance my Active Record validations were not showing error messages when they were being validated. I ended modifying the create action in my own UsersController so that it always shows at least one of the errors:
def create
@user = user_from_params
if @user.save
sign_in @user
redirect_back_or url_after_create
else
flash.now[:error] = @user.errors.full_messages[0]
render template: "users/new"
end
end
Hopefully this applies to this discussion!
Although the solution is easy for customized signup process, but there's no code comments or docs saying please add it yourself 😓 although it should be obvious, but still, it would be nice to remind devs that there will be no flashing errors there.