leastbad / optimism

The missing drop-in solution for realtime remote form validation in Rails.
MIT License
361 stars 42 forks source link

Adds a raw error message config option #15

Open mepatterson opened 4 years ago

mepatterson commented 4 years ago

In the case where you might want Optimism to only send the literal error message string (without the attribute name humanized in front of it), you need to not go through full_messages since that prefixes the error message with the attribute.

This PR adds an optional config switch (default: false) to tell Optimism to just send the 'raw' error message for an invalid attribute. (e.g. instead of sending "Name is too short" it will send "is too short" which can be useful if the consumer is manually constructing their own messages.

leastbad commented 4 years ago

Hey, thanks for this - it's interesting.

First, I need you to read through this PR: https://github.com/leastbad/optimism/pull/4

Long story short, I learned the hard way that using full_message impacts more than just the string we perceive.

Now, I'm open to the idea that not displaying the full message could be really helpful but I admit I'm skeptical that even you want it that way for every field. This is such an all-or-nothing approach, after all.

I did have an alternative idea. I don't know how much merit it has. What if instead of omitting the attribute from the string, we wrapped the attribute in a span and put a class on it? Then you could display: none that class on a per-attribute basis and not remove the attribute entirely. Could something like that work?

mepatterson commented 4 years ago

Yeah, I had considered that a sledgehammer approach like I did would maybe not be the best. Your idea about returning it as HTML with some classes that can be hidden is interesting.

something like: <span class='form-attr'>Name</span>is too short

I wonder, though, if it could end up hurting someone later who happens to be trying to use that exact same class for something else?

leastbad commented 4 years ago

We could create a configurable property for that class...

mepatterson commented 4 years ago

That would work... On Jul 7, 2020, 8:59 AM -0500, leastbad notifications@github.com, wrote:

We could create a configurable property for that class... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

leastbad commented 4 years ago

Hey Matt, is this still something that's making life hard for you? I haven't had time to approach creating a configurable property for this, but I will if you need it.

mepatterson commented 4 years ago

I hacked around it for now. I need to investigate how it plays with i18n also. That might be a better solution to rewriting the messages anyway On Jul 22, 2020, 12:46 PM -0500, leastbad notifications@github.com, wrote:

Hey Matt, is this still something that's making life hard for you? I haven't had time to approach creating a configurable property for this, but I will if you need it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

leastbad commented 4 years ago

Okay, I am going to stop thinking about this again, then. Too many things! :)

Ping me if you want to revisit.

Inkybro commented 4 years ago

I haven't taken much time to go through the code so forgive me if this is way off-base, but why not have this as an option passed into error_for, e.g. <%= form.error_for :username, raw_message: true %>?

Would this not be a more acceptable solution than CSS hacks or global configuration options?

I found myself in need of exactly this functionality just now which is what lead me here. I have an Email model with an address field and I use nested attributes for it, which makes the error messages come up as "Address ...".

A suitable alternative, at least in my case, would be an option like <%= nested_email.error_for :address, attribute_name: 'Email address' %>

It seems both of these options would be really useful to someone, somewhere, in some case. So maybe an option attribute_name that can be set to either false or a string/symbol, where false would give back the raw message and string/symbol would replace the attribute name?

Inkybro commented 4 years ago

Ah, I see that wouldn't work so easily. Hmm...

Inkybro commented 4 years ago

Scratch all of that, I just realized the behavior I needed already exists. I had never really messed with I18n before today so this ended up being a nice learning experience. Apologies for cluttering things up.

Inkybro commented 4 years ago

@mepatterson I know you worked around your issue for now, but does #18 seem to solve that?