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

Support runtime recomputing error messages #12

Closed
jasonmit opened this issue Mar 14, 2016 · 11 comments
Closed

Support runtime recomputing error messages #12

jasonmit opened this issue Mar 14, 2016 · 11 comments

Comments

@jasonmit
Copy link
Owner

jasonmit commented Mar 14, 2016

No description provided.

@jelhan
Copy link
Contributor

jelhan commented May 23, 2016

Could add i18n.locale to dependentKeys as a work-a-round. Make sure i18n.locale gets consumed.

const Validations = buildValidations({
  title: [
    validator('length', {
      dependentKeys: ['i18n.locale'],
      min: 2
    })
  ]
});

export default Ember.Controller.extend(Validations, {
  init() {
    this.get('i18n.locale');
  },
  i18n: Ember.inject.service()
});

@jasonmit
Copy link
Owner Author

Yup, was opened prior to this being supported. Closing now but thanks for adding details since others may not known!

@jelhan
Copy link
Contributor

jelhan commented May 24, 2016

@jasonmit native support would be fine anyway. My work-a-round is far from being optimal: Property is revalidated if locale changes and a lot of boilerplate code is needed. I think we should leave this one open as a feature request.

@jelhan
Copy link
Contributor

jelhan commented May 24, 2016

If you override getDependentsFor method of validators and add _model.i18n.locale you don't have to add i18n.locale over and over again to dependentKeys:

import Base from 'ember-cp-validations/validators/base';

Base.reopenClass({
  getDependentsFor() {
    return ['_model.i18n.locale'];
  }
});

You still have to inject i18n service to model and consume i18n.locale.

@jasonmit
Copy link
Owner Author

Unsure I want to go down that road, that's making the assumption that everyone injects the service with the i18n key. While it's likely true for most, to me it doesn't feel right. I'm opened to exploring ways to make this integration easier though, just unsure what that would look like.

Reopening for discussion

@jasonmit jasonmit reopened this May 24, 2016
@jasonmit jasonmit changed the title Changing locale at runtime should result in errors being recomputed Support runtime recomputing error messages May 24, 2016
@jelhan
Copy link
Contributor

jelhan commented May 25, 2016

It's not more than this changes to ember-cp-validations needed: jelhan/ember-cp-validations@eeac8b5 Of course a refactoring would be necessary to let us override these internals.

@jasonmit
Copy link
Owner Author

jasonmit commented May 25, 2016

Some coordination with @offirgolan would be necessary to expose that encapsulated type the factory method returns.

@offirgolan
Copy link
Contributor

If you override getDependentsFor method of validators and add _model.i18n.locale you don't have to add i18n.locale over and over again to dependentKeys

This solution will not work since any validator that has dependents will override the base getDependentsFor method.

What I could do is always concat the contents of the dependents in base with every single validator's returned dependents which will allow you to add the i18n service dependent.

You still have to inject i18n service to model and consume i18n.locale.

In terms of service injection, I think the best solution would be to add documentation to tell users they have to inject the service in their model/component/etc. I dont really like the idea of forcefully injecting services.

@jasonmit
Copy link
Owner Author

I share this same opinion that this is best solved with documentation

@offirgolan
Copy link
Contributor

Let me know if you want me to work on my first point or if you just prefer to add more documentation telling users to add the necessary dependentKeys to their global options.

@jasonmit
Copy link
Owner Author

adopted-ember-addons/ember-cp-validations#146 resolved this with the ability to add "global" dependentKeys preventing having to add i18n.locale to each validation.

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

No branches or pull requests

3 participants