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

Better error messages for mdl-textfield #65

Merged
merged 4 commits into from Nov 27, 2015

Conversation

kennethkalmer
Copy link
Contributor

I added support for binding errorMessage to something coming from outside the component. Lets say you have existing validations or server-side validations and want to provide feedback to the user, the simple pattern approach provided by the HTMLInputElement doesn't work.

This approach simply observes the errorMessage property, sanitises the value slightly, and calls the HTMLInputElement's setCustomValidity, which in turns triggers MDL to do its thing and apply the correct is-invalid class to the wrapping div...

Pending initial feedback I'll get this implemented on {{mdl-textarea}} too.

@mike-north
Copy link
Owner

This looks great. Let's get it implemented with mdl-textarea as well in this PR, so we can have one release with consistent support for error messages

@@ -12,5 +18,15 @@ export default BaseComponent.extend({
],
beforeMdlInit() {
this.$('label.mdl-button').attr('for', this.get('_inputId'));
}
},
setValidity: observer('errorMessage', function() {
Copy link
Owner

Choose a reason for hiding this comment

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

please indicate that this is private (_setValidity)

@kennethkalmer
Copy link
Contributor Author

@mike-north thanks for the feedback, will roll with this and get it done by the weekend!

@alanpeabody
Copy link

This is great, looking forward to using it.

To hijack this PR a bit: Would formatting ember-data error messages directly be something you are interested in supporting?

For example, given post.errors is an DS.Errors object (http://emberjs.com/api/data/classes/DS.Errors.html):

{{mdl-textarea
  label='Title'
  value=post.title
  errors=post.errors.title
}}

Or maybe I should just use a sub-expression?

Added integration tests for textarea
Moved common input validations to base-input-component
@kennethkalmer
Copy link
Contributor Author

@mike-north implemented the textarea part too, apologies for the delays.

@alanpeabody, imho I don't think it is a good idea. Just pass in a computed property with the error, we can't favour one style of validations over another.

@alanpeabody
Copy link

@mike-north @kennethkalmer Any update if this will be merged or not?

@20v100
Copy link

20v100 commented Nov 1, 2015

Looking forward for the merge.

mike-north added a commit that referenced this pull request Nov 27, 2015
Better error messages for mdl-textfield
@mike-north mike-north merged commit f744e75 into mike-north:master Nov 27, 2015
@coveralls
Copy link

Coverage Status

Coverage increased (+11.2%) to 100.0% when pulling fc74171 on kennethkalmer:better-error-messages into 61ac1ea on mike-north:master.

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.

None yet

6 participants