I added a config option to show attribute names by errors #922

Closed
wants to merge 9 commits into
from

Projects

None yet

3 participants

@michaelcarter

This is a follow up to my issue post: #919

If this gets accepted I'll include the functionality in formtastic-bootstrap ASAP and put in another pull request for that. I've tried my best to do this touching as little code as possible.

@justinfrench
Owner

I think errors() should not take an options hash. You have access to the builder/configuration, and everywhere you're calling errors has been changed to errors(error_options), so moving to an options hash feels unnecessary from what I've seen. If it becomes necessary, I'd prefer to see a separate method.

I think it'd also be helpful for us in the future to document the reason for adding this too (in this PR will do), for future reference. Is it to satisfy a Bootstrap pattern, or just personal preference?

@michaelcarter

It's a bit of both if I'm honest, I tend to set the error text to be a bit smaller than the input labels, and it just looks untidy to me that it doesn't show the input name next to the error, to the point that I was convinced it wasn't intentional.

I'll remove the options hash, I had passed it along to avoid appending the attribute names on all errors if we were just taking the first, but I guess it introduces more complexity than it cuts down on overhead. Working on an updated commit now.

@msaspence

For me this makes sense because I have design where the label has been hidden in the design.

@justinfrench
Owner

This needs some spec coverage, but I'm happy to merge it in after that. Will close down if there's no activity after a month or so.

@justinfrench
Owner

Closing due to inactivity, happy to merge with spec coverage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment