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

[Feature] Warning Validators #226

Merged
merged 5 commits into from
Jun 27, 2016
Merged

[Feature] Warning Validators #226

merged 5 commits into from
Jun 27, 2016

Conversation

offirgolan
Copy link
Collaborator

@offirgolan offirgolan commented Jun 23, 2016

Use Case:

You have a password field with a length validator of { min: 4, max: 10 } but want to display a warning telling the user their password is hella weak if its under 6 characters.

Solution:

Warning validators!

How it works:

  • They should be declared like any other validator
  • Act like an assert. If the conditions are unsatisfied, should return an error message (just like any validator)
  • These validators will be filtered out of the "required" validators and all messages will be put in the warnings collection.
  • It should be up to the user when to display those warnings but they will not have any effect on the overall valid state of the attribute.

Usage:

password: {
  description: 'Password',
  validators: [
    validator('length', {
      min: 4,
      max: 10
    }),
    validator('length', {
      isWarning: true,
      min: 6,
      message: 'What kind of weak password is that?'
    })
  ]
}

screen shot 2016-06-23 at 4 19 10 pm

@rwjblue @stefanpenner @blimmer I would love to hear some input from your guys. Questions, concerns, whatever

@offirgolan
Copy link
Collaborator Author

offirgolan commented Jun 25, 2016

Questions that need answering:

  1. Is warning the right property to have? Maybe required or severity?
  2. Which attributes in result-collection.js should be dependent on required validations or on all validations? From what I feel like, all promise related attribute should be dependent on all validators, that way they all resolve together?

}).readOnly(),

warning: computed('warning.[]', cycleBreaker(function () {
return get(this, 'warning.0');
Copy link
Collaborator

Choose a reason for hiding this comment

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

is .0 the right thing to use here? I've always used firstObject.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the same code that is the errors and error CPs. I believe I tried firstObject but it errored on me. Ill give it another shot though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No problem - definitely not required, I just haven't seen this syntax before.

Copy link
Collaborator

Choose a reason for hiding this comment

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

firstObject ideally

@offirgolan offirgolan modified the milestone: 3.0 Jun 27, 2016
* @readOnly
* @type {Ember.ComputedProperty | Error}
*/
warning: computed('warnings.[]', cycleBreaker(function () {
Copy link
Collaborator

Choose a reason for hiding this comment

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

firstObject

@stefanpenner
Copy link
Collaborator

stefanpenner commented Jun 27, 2016

thoughts on warning: true -> isWarning: true?

@offirgolan
Copy link
Collaborator Author

@stefanpenner 👍

@offirgolan offirgolan merged commit d94917f into master Jun 27, 2016
@offirgolan offirgolan deleted the warnings branch June 27, 2016 22:29
offirgolan added a commit that referenced this pull request Jun 27, 2016
* master:
  [Feature] Warning Validators (#226)
  Evaluate attrs options hash (#223)
  chore(package): update ember-try to version 0.2.4 (#221)
  Fix for data validator with format (#218)
  chore(package): update ember-cli to version 2.6.2 (#217)
  chore(package): update ember-cli-moment-shim to version 2.0.0 (#216)
  chore(package): update ember-cli-blanket to version 0.9.5 (#214)
  Add import for getOwner to README example (#215)

# Conflicts:
#	addon/validations/result-collection.js
#	addon/validations/result.js
#	tests/integration/validations/factory-general-test.js
offirgolan added a commit that referenced this pull request Jun 28, 2016
* master:
  [Feature] Validate Attribute (#235)
  [Feature] Warning Validators (#226)

# Conflicts:
#	tests/integration/validations/factory-general-test.js
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

3 participants