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

validation for autocomplete and change to form #532

Merged
merged 2 commits into from
Oct 31, 2016

Conversation

ibarrick
Copy link
Contributor

Added contextual components to paper-form for select and autocomplete. Interesting issue with autocomplete is that there are two properties that we may want to watch for validation over but currently validation-mixin only supports a single validation property. For now I set it to do validation on searchText but that can be changed by the user by passing in validationProperty='selected'.

Validation messages for autocomplete will only show up for the floating label version for obvious reasons.

@ibarrick
Copy link
Contributor Author

This should be set to go now.

@miguelcobain miguelcobain merged commit 05414b5 into adopted-ember-addons:master Oct 31, 2016
@miguelcobain
Copy link
Collaborator

@ibarrick
Copy link
Contributor Author

@miguelcobain I don't think so, that should be taken care of by the paper-input component that the trigger renders if label is defined. We only had to do that in paper-select because it no longer extends paper-input.

@miguelcobain
Copy link
Collaborator

miguelcobain commented Oct 31, 2016

🤔 hmmm

Why then does paper-complete need ValidationMixin in the first place?
It doesn't seem to do a lot of what it should. selected and searchtext are never validated, it seems to me. There is no way to render errors for those.

@ibarrick
Copy link
Contributor Author

I see what you're saying. I can pass validationErrorMessages down to the paper-input instead of required and customValidations and it should display the error messages as if they originated from the paper-input.

@miguelcobain
Copy link
Collaborator

That makes sense.
Also, with that scenario, paper-autocomplete doesn't exactly need the validationmixin?

@ibarrick
Copy link
Contributor Author

In that scenario it would because the validationErrorMessages array is computed by the mixin and should be computed for searchTerm or selected, rather than the value of the input.

We could remove the validation mixin right now if we wanted but I think that would be a mistake since it is conceivable that a user may want to do more complicated validation on selected, such as computing the validation based on properties of selected other than what is rendered in the paper-input.

@miguelcobain
Copy link
Collaborator

I see. So we would be essentially computing the errors on autocomplete and passing them to paper-input for it to render.
I wouldn't want paper-input to compute any kind of validation. validationProperty=null doesn't seem possible because we assert its presence.

@ibarrick
Copy link
Contributor Author

not passing in any validations should ensure that it doesn't compute any of it's own it should just render the messages it's given

@miguelcobain
Copy link
Collaborator

Great. So it seems we have a path forward.
Unfortunately I've already merged this, so we need a new PR.

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.

2 participants