Skip to content

Conversation

ale-rt
Copy link
Member

@ale-rt ale-rt commented Jul 23, 2020

This is particularly interesting for password or email confirmation fields

@ale-rt ale-rt requested review from pilz and thet July 23, 2020 09:27
@cornae
Copy link
Member

cornae commented Jul 23, 2020

Very useful!

This is particularly interesting for password or email confirmation
fields
@ale-rt ale-rt force-pushed the pat-validation-equality branch from 95ef6aa to ec5c5a3 Compare July 23, 2020 09:52
Copy link
Member

@thet thet left a comment

Choose a reason for hiding this comment

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

Some minor changes requested..., otherwise LGTM

| Property | Description | Default | Type |
|-------------------|------------------------------------------------------------------------------------|---------|------|
| disable-selector | A selector for elements that should be disabled when there are errors in the form. | | CSS Selector |
| equality | Field-specific. The name of another input this input should equal to (useful for password confirmation). | | String |
Copy link
Member

Choose a reason for hiding this comment

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

Please also add an entry for message-equality.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added!

this.$el.find("[name=" + opts.equality + "]").each(
function (idx, el) {
if (input.value !== el.value) {
constraint.equality = {'attribute': opts.equality, 'message': '^'+opts.message["equality"]};
Copy link
Member

Choose a reason for hiding this comment

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

In my understanding of http://validatejs.org/#validators-equality the attribute attribute shouldn't be neccessary.

Copy link
Member

Choose a reason for hiding this comment

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

Do you know what the '^ is for? it looks strange, but I see all the other message-* attributes have it too.

Copy link
Member Author

Choose a reason for hiding this comment

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

In my understanding of http://validatejs.org/#validators-equality the attribute attribute shouldn't be neccessary.

If I remove it the tests fail. I think attribute is mandatory when you also specify a message.

Do you know what the '^ is for? it looks strange, but I see all the other message-* attributes have it too.

No idea, I put it there to be consistent with the already existent code :)

}

// Handle fields equality
if (opts.equality && input.value) {
Copy link
Member

@thet thet Jul 24, 2020

Choose a reason for hiding this comment

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

I think the && input.value check should be removed. You probably still want to check for equality if no value was given. Instead of checking for && input.value we can enforce presence via the required attribute.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the check

@ale-rt
Copy link
Member Author

ale-rt commented Jul 27, 2020

I implemented all the requested changes except #740 (comment) because either I did not understand how to do it or it is not possible to omit the attribute once a message is specified.
So this one is ready to be rereviewed.

@pilz pilz merged commit 21a78ae into master Jul 29, 2020
@pilz pilz deleted the pat-validation-equality branch July 29, 2020 12:27
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.

4 participants