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

"validatePresence on" not updating #193

Open
Redsandro opened this issue Feb 27, 2019 · 7 comments
Open

"validatePresence on" not updating #193

Redsandro opened this issue Feb 27, 2019 · 7 comments

Comments

@Redsandro
Copy link
Contributor

validatePresence({ presence: true, on: 'email' })

Version

2.1.0

This is probably me misunderstanding the behavior of the validator.

Steps to reproduce

export default {
	email: [
		validatePresence(true),
		validateFormat({ type: 'email' }),
	],
	password: [
		validatePresence({ presence: true, on: 'email' }),
	],
}

Expected Behavior

Empty password validates fine, but

  1. turns invalid once an email is entered, or
  2. turns invalid after focused after email is entered.

Actual Behavior

Empty password stays valid, focused or not.
Once you type in a password and then remove it again then the validator fails (as expected).

@snewcomer
Copy link
Collaborator

From my pov, 1 && 2 are features that some/many users wouldn't want since eagerly displaying error messages would be optically annoying. What do you think? If you really wanted this behaviour, we could try to figure out something, but would have to be implemented on your end. Lmk your thoughts!

@Redsandro
Copy link
Contributor Author

@snewcomer I'm experimenting a bit as I've run into limitations with cp-validations and am looking to replace it. I don't currently need this, and I agree with the optical annoyance in this scenario. However, doesn't this mean that the on: functionality is broken nonetheless?

Perhaps a better example would be an address with streetname, apt number, and postal code. The address is optional, but if the user enters the streetname e.g. for future deliveries, you want apt number and postal code to no longer allow empty.

However, if either apt number or postal code had been previously (accidentally) focused, they will stay validated and accepted as empty. They will stay green, and only on form submit will those fields (provided they are given the allowEmpty=false treatment) popup a tool tip saying "This field cannot be empty" (while still being marked as accepted in the template). That's optically confusing.

Now I can hide the apt number and postal code until streetname is no longer empty, but it would be a workaround because the on: is not really doing what I would expect.

I am open to the possibility that I am misunderstanding the use cases of 'on:'. ☺️

@allthesignals
Copy link

I don't see on documented anywhere 🤔 https://offirgolan.github.io/ember-validators/docs/classes/Presence.html

I'd like to validate presence conditionally.

@Redsandro
Copy link
Contributor Author

@allthesignals Not sure what is the latest trend, but this issue was based on version 2.1.0. The expected validatePresence behavior was documented in this #191 based on issue #189.

@snewcomer
Copy link
Collaborator

@Redsandro In response to this issue, if you had a minimal reproduction that I could take a look at, I think we could close this issue once and for all 👍

@Redsandro
Copy link
Contributor Author

Redsandro commented May 9, 2020

@snewcomer unfortunately I'm not using this currently.

The issue was that on wasn't working as advertised for validatePrecense. It was more than a year ago. I'm not sure the feature is officially supported anymore.

We can close this, although @allthesignals seems to be looking for exactly this feature. Perhaps these days there's a different approach to achieving conditional precense.

@allthesignals
Copy link

allthesignals commented May 10, 2020

I've been working on something that could achieve this, but I'm having trouble with the re-validation issue you're seeing.

My use case is conditional presence validation but the condition is more specific: it should only validate presence when the value of the on field equals something specific.

This has been a matter of updating these lines to something like this, and it works okay:

   const hasTargetValues = targets.some((target) => changes[target] || (changes[target] === undefined && content[target]));
    const hasMatchingWith = options.withValue && targets.some((target) => (changes[target] || content[target]) === options.withValue);

    if ((targets && !hasTargetValues) || !hasMatchingWith) {
      return true;
    }

I forked the original twiddle, updated to the latest ember/addon: https://ember-twiddle.com/40a98999c925a2c81ffe87882585de0f?openFiles=validations.user.js%2C

You can see that it's necessary to focus into the conditionally validated field to get it to validate. With input fields, this isn't as much of a problem for the user because they can just click it. It will be confusing but not a disaster.

However, if you're showing/hiding the conditional validated field, it's a problem because there's no way to re-validate everything.

@snewcomer @Redsandro I'm curious if you can think of an approach to get around this problem? If we can solve it as-is with the addon, that would also help with the extended behavior I'm working towards.

Is there some way to get it to re-validate anything specified in on? I don't think validators will have access to that sort of API...

EDIT:
Here's a nicer implementation of the "validatePresenceIf" behavior which actually builds on top of the current validatePresence :)

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

No branches or pull requests

3 participants