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

Calling Form->input() with 'error'=>'' should trigger field error, but not render error element. #918

Merged
merged 1 commit into from
Dec 1, 2012

Conversation

bar
Copy link
Contributor

@bar bar commented Oct 25, 2012

From #863 (comment)

The idea is to complete [1] and [2] which enabled to trigger element errors (field coloring) but not rendering error messages.

Actually, calling Form->input() with:
'error' => false prevent any kind of error reporting (field error coloring AND error messages)
'error' => '' allow field error coloring AND render an HTML element with and empty error message

IMO,
'error' => '' should allow field error coloring BUT prevent rendering an HTML element with and empty error message.

[1] 9f23f65
[2] ae8dd1c

@markstory
Copy link
Member

Any objections to this? I think its reasonable for 2.3 It adds a bit more sensitive to the error key and empty values, but its already kind of complicated.

@lorenzo
Copy link
Member

lorenzo commented Nov 3, 2012

That is kind of complicated and magic I would never use an empty string as a configure value for anything. Is there a way we can change it to something more meaningful?

@bar
Copy link
Contributor Author

bar commented Nov 3, 2012

@lorenzo you mean, using two variables?

@lorenzo
Copy link
Member

lorenzo commented Nov 3, 2012

Yeah, that might be the only solution without making option handling complicated

@bar
Copy link
Contributor Author

bar commented Nov 3, 2012

👍 for that

I'm thinking something like:
'error' => false prevent any kind of error reporting (field error coloring AND error messages)
'errorMessage' => false (default: true) allow field error coloring BUT prevent rendering an HTML element with an empty error message.

@markstory
Copy link
Member

I like the idea of an additional option as well. This makes the intent clearer and doesn't add additional behavior to various falsey values.

@bar
Copy link
Contributor Author

bar commented Nov 16, 2012

Rebased to 2.3 and updated as discussed :)

@bar
Copy link
Contributor Author

bar commented Nov 16, 2012

I'll update the docs if this goes through.

* - `options` - For widgets that take options e.g. radio, select.
* - `error` - Control the error message that is produced. Set to `false` to disable any kind of error reporting (field
* error coloring and error messages).
* - `errorMessage` - Boolean to control rendering error messages (field error coloring will still occur).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"coloring" is not an accurate word. The wrapper div gets the error class, you could have any styling for that class.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh man, feature blindness on that one!
Fixed

error, but not render error message (HTML element).
@bar
Copy link
Contributor Author

bar commented Nov 16, 2012

Updated again, missed a 'color' :P

@lorenzo
Copy link
Member

lorenzo commented Dec 1, 2012

I think this makes sense, merging this one

lorenzo added a commit that referenced this pull request Dec 1, 2012
Calling Form->input() with 'errorMessage'=> false should trigger field error, but not render error element.
@lorenzo lorenzo merged commit dc37082 into cakephp:2.3 Dec 1, 2012
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.

4 participants