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

ActiveForm: Trigger `onValidationError` event on input & form #4955

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@kartik-v
Contributor

kartik-v commented Sep 8, 2014

Trigger a new onValidationError event on both input & form, whenever a validation error is encountered. This can help one to code advanced stuff like:

$form.on('onValidationError', function(event, attribute, messages) {
    if (messages.length && attribute.id == 'attribute1') {
        // do something
    } else if (messages.length) {
        // do something else
    }
});
// or with each input
$input.on('onValidationError', function(event, messages) {
    if (messages.length) {
        // do something
    }
});
Trigger `onValidationError` event on input & form
Trigger a new `onValidationError` event on both input & form, whenever a validation error is encountered. This can help one to code advanced stuff like:
```js
$form.on('onValidationError', function(event, messages) {
    if (messages.length) {
        // do something
    }
});
```

@kartik-v kartik-v changed the title from Trigger `onValidationError` event on input & form to ActiveForm: Trigger `onValidationError` event on input & form Sep 8, 2014

@kartik-v kartik-v referenced this pull request Sep 8, 2014

Closed

yii\widgets\ActiveForm #1350

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

So the event is triggered for each error input, and when any input has error?
Should they trigger different events? And for the former, should it include which attribute is having error?

Member

qiangxue commented Sep 8, 2014

So the event is triggered for each error input, and when any input has error?
Should they trigger different events? And for the former, should it include which attribute is having error?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

Yes the idea is to be able to detect any instance of error received either across the entire form OR if needed separately for specific inputs.

Contributor

kartik-v commented Sep 8, 2014

Yes the idea is to be able to detect any instance of error received either across the entire form OR if needed separately for specific inputs.

@qiangxue qiangxue added this to the 2.0 RC milestone Sep 8, 2014

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

And for the former, should it include which attribute is having error

Yes we can do that... updating

Contributor

kartik-v commented Sep 8, 2014

And for the former, should it include which attribute is having error

Yes we can do that... updating

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

The attribute which raised the error can be captured at form level like this...

$form.on('onValidationError', function(event, attribute, messages) {
    if (messages.length && attribute.id == 'attribute1') {
        // do something
    } else if (messages.length) {
        // do something else
    }
});
Contributor

kartik-v commented Sep 8, 2014

The attribute which raised the error can be captured at form level like this...

$form.on('onValidationError', function(event, attribute, messages) {
    if (messages.length && attribute.id == 'attribute1') {
        // do something
    } else if (messages.length) {
        // do something else
    }
});
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 8, 2014

Member

imo there should be different names for the events.

  • onValidationError - called for the whole form
  • onAttributeValidationError - called on every attribute
Member

cebe commented Sep 8, 2014

imo there should be different names for the events.

  • onValidationError - called for the whole form
  • onAttributeValidationError - called on every attribute
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Perhaps using the same name is fine. We can use function (event, messages, attribute) as the function signature, and if attribute is undefined, it means the event is for the whole form.

Member

qiangxue commented Sep 8, 2014

Perhaps using the same name is fine. We can use function (event, messages, attribute) as the function signature, and if attribute is undefined, it means the event is for the whole form.

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

The event is triggered separately on different elements form and input. So that itself is a key differentiator even though event names are same. Something like $('form').on('change') and S('input').on('change').

Contributor

kartik-v commented Sep 8, 2014

The event is triggered separately on different elements form and input. So that itself is a key differentiator even though event names are same. Something like $('form').on('change') and S('input').on('change').

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Yeah, I didn't notice that initially.

So this event is triggered mainly to do some extra work in addition to the existing error display action? Is there any need to replace or prevent the default error display?

Member

qiangxue commented Sep 8, 2014

Yeah, I didn't notice that initially.

So this event is triggered mainly to do some extra work in addition to the existing error display action? Is there any need to replace or prevent the default error display?

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 8, 2014

Member

The event is triggered separately on different elements form and input.

yeah right, did not notice that too :)

Member

cebe commented Sep 8, 2014

The event is triggered separately on different elements form and input.

yeah right, did not notice that too :)

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

So this event is triggered mainly to do some extra work in addition to the existing error display action?

Absolutely.

Is there any need to replace or prevent the default error display?

I originally (in my comment in #1350) had thought if there could be a beforeValidationError event that allows you to control displaying the error. But at least this is an important first step to detect any error is raised.

This is so useful for custom jquery plugins or ActiveForm related extensions OR ajax based submits - to trap model validation errors on client and abort any save/submission if any validation error is encountered.

Contributor

kartik-v commented Sep 8, 2014

So this event is triggered mainly to do some extra work in addition to the existing error display action?

Absolutely.

Is there any need to replace or prevent the default error display?

I originally (in my comment in #1350) had thought if there could be a beforeValidationError event that allows you to control displaying the error. But at least this is an important first step to detect any error is raised.

This is so useful for custom jquery plugins or ActiveForm related extensions OR ajax based submits - to trap model validation errors on client and abort any save/submission if any validation error is encountered.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

If we were to support beforeValidationError, should it be done in terms of an event or a callback? Note also that beforeValidate, beforeValidateAll, etc. are all callbacks. So a more general question is: should we use callbacks or events?

Member

qiangxue commented Sep 8, 2014

If we were to support beforeValidationError, should it be done in terms of an event or a callback? Note also that beforeValidate, beforeValidateAll, etc. are all callbacks. So a more general question is: should we use callbacks or events?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

Yes I think beforeValidate was added and should be fine for handling the beforeValidationError scenarios.

The advantage of events vs callbacks - is when you want to at runtime understand if there is a validation error through other extensions/plugins on the same page.

This is assuming an use case --- where you cannot set the beforeValidate callback from PHP as required at ActiveForm init but instead will be based on dynamic conditions at runtime. However, having said that, I think at this stage the beforeValidate callback and onValidationError event in combination should suffice handling most of such scenarios.

Contributor

kartik-v commented Sep 8, 2014

Yes I think beforeValidate was added and should be fine for handling the beforeValidationError scenarios.

The advantage of events vs callbacks - is when you want to at runtime understand if there is a validation error through other extensions/plugins on the same page.

This is assuming an use case --- where you cannot set the beforeValidate callback from PHP as required at ActiveForm init but instead will be based on dynamic conditions at runtime. However, having said that, I think at this stage the beforeValidate callback and onValidationError event in combination should suffice handling most of such scenarios.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Thanks for the explanation. So do you think we should consider turning beforeValidate etc. into events as well?

Member

qiangxue commented Sep 8, 2014

Thanks for the explanation. So do you think we should consider turning beforeValidate etc. into events as well?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

So do you think we should consider turning beforeValidate etc. into events as well?

Yes IMO... that may give better control to the developer and can be used in more scenarios including what it does now. Some more flags may need to be captured in the activeform plugin to detect a few statuses to optimize.

Contributor

kartik-v commented Sep 8, 2014

So do you think we should consider turning beforeValidate etc. into events as well?

Yes IMO... that may give better control to the developer and can be used in more scenarios including what it does now. Some more flags may need to be captured in the activeform plugin to detect a few statuses to optimize.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Thanks. @yiisoft/core-developers What's your opinion? While this will cause BC breaks, I think it's worthwhile in the long run by changing callbacks to events.

Member

qiangxue commented Sep 8, 2014

Thanks. @yiisoft/core-developers What's your opinion? While this will cause BC breaks, I think it's worthwhile in the long run by changing callbacks to events.

@samdark

This comment has been minimized.

Show comment
Hide comment
@samdark

samdark Sep 8, 2014

Member

Worth doing.

Member

samdark commented Sep 8, 2014

Worth doing.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 8, 2014

Member

where would I listen to the event then? How does the usage of JS code configured in the ActiveForm class change?

Member

cebe commented Sep 8, 2014

where would I listen to the event then? How does the usage of JS code configured in the ActiveForm class change?

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

The form object. You won't be able to configure JS via ActiveForm.

Member

qiangxue commented Sep 8, 2014

The form object. You won't be able to configure JS via ActiveForm.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 8, 2014

Member

k

Member

cebe commented Sep 8, 2014

k

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

You would listen to all events on the form or the input element.

$('form').on('beforeValidation', function(event, params) {
   // do stuff
});
// similarly could trigger afterValidation or other events
Contributor

kartik-v commented Sep 8, 2014

You would listen to all events on the form or the input element.

$('form').on('beforeValidation', function(event, params) {
   // do stuff
});
// similarly could trigger afterValidation or other events
@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 8, 2014

Member

btw, the form properties could still register JS code for event handling. Not sure if it is needed though...

Member

cebe commented Sep 8, 2014

btw, the form properties could still register JS code for event handling. Not sure if it is needed though...

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

btw, the form properties could still register JS code for event handling.

Yes, but we probably should not support this because it means mixing js code with PHP code.

Member

qiangxue commented Sep 8, 2014

btw, the form properties could still register JS code for event handling.

Yes, but we probably should not support this because it means mixing js code with PHP code.

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

I actually want to trigger all events on the form.

We could do that. As in this PR example, it returns the attribute config for callback in the form('onValidationError') event. But with the current implementation, since its within a loop, I suppose the same event is triggered multiple times on the form element for each attribute (for this current PR). An improvement option can be to stack all the attribute validation results in an array and trigger the event only once (outside the loop) and pass the array of validation attributes as params to the event.

Contributor

kartik-v commented Sep 8, 2014

I actually want to trigger all events on the form.

We could do that. As in this PR example, it returns the attribute config for callback in the form('onValidationError') event. But with the current implementation, since its within a loop, I suppose the same event is triggered multiple times on the form element for each attribute (for this current PR). An improvement option can be to stack all the attribute validation results in an array and trigger the event only once (outside the loop) and pass the array of validation attributes as params to the event.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

I suppose the same event is triggered multiple times on the form element for each attribute (for this current PR).

Is this a problem?

Member

qiangxue commented Sep 8, 2014

I suppose the same event is triggered multiple times on the form element for each attribute (for this current PR).

Is this a problem?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

I suppose it is a problem - because it would cause the n number of multiple calls (this may be redundant and not good for performance when condition may be exactly same). But it can be overcome with a bit of code at runtime. I suppose it may not be clean though. Let me explain:

  1. Let's say the form contains 10 attributes and all attributes have a validation error.
  2. There will be minimal 10 events raised for onValidationError if its within a loop (assuming one validation error per attribute).
  3. If called at runtime without any condition, this will fire 10 different javascript calls.
  4. It can be overcome to some extent by doing this:
$('form').on('onValidationError', function(event, attribute, messages) {
    if (messages.length && attribute.id == 'attribute1') {
        // this will fire only once for the attribute - still 10 different times this validation will be checked
    } else if (messages.length) {
        // this will fire 10 different javascript calls probably for the same condition
    }
});

A cleaner approach can be to set all validation errors in an array outside the loop so something like this:

$('form').on('onValidationError', function(event, results) {
    if (results.length && results.attributes.length) {
         var attribs = results.attributes, j = 0, errors = [];
         for (var i = 0; i < attribs.length; i++) {
             if (attribs[i]['id'] == 'attribute1' || attribs[i]['id'] == 'attribute2' || attribs[i]['id'] == 'attribute3') {
                  errors[j] = attribs[i]['errors']; // attribute specific errors
                  j++;
             }
         }

         if (errors.length) {
             // do something
         }
    } else if (results.length) {
         var errors = results.errors; // all errors
        // still will fire only once
    }
});
Contributor

kartik-v commented Sep 8, 2014

I suppose it is a problem - because it would cause the n number of multiple calls (this may be redundant and not good for performance when condition may be exactly same). But it can be overcome with a bit of code at runtime. I suppose it may not be clean though. Let me explain:

  1. Let's say the form contains 10 attributes and all attributes have a validation error.
  2. There will be minimal 10 events raised for onValidationError if its within a loop (assuming one validation error per attribute).
  3. If called at runtime without any condition, this will fire 10 different javascript calls.
  4. It can be overcome to some extent by doing this:
$('form').on('onValidationError', function(event, attribute, messages) {
    if (messages.length && attribute.id == 'attribute1') {
        // this will fire only once for the attribute - still 10 different times this validation will be checked
    } else if (messages.length) {
        // this will fire 10 different javascript calls probably for the same condition
    }
});

A cleaner approach can be to set all validation errors in an array outside the loop so something like this:

$('form').on('onValidationError', function(event, results) {
    if (results.length && results.attributes.length) {
         var attribs = results.attributes, j = 0, errors = [];
         for (var i = 0; i < attribs.length; i++) {
             if (attribs[i]['id'] == 'attribute1' || attribs[i]['id'] == 'attribute2' || attribs[i]['id'] == 'attribute3') {
                  errors[j] = attribs[i]['errors']; // attribute specific errors
                  j++;
             }
         }

         if (errors.length) {
             // do something
         }
    } else if (results.length) {
         var errors = results.errors; // all errors
        // still will fire only once
    }
});
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Here's my thought:

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
         // handle for the whole form (only once)
    } elseif (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
});
Member

qiangxue commented Sep 8, 2014

Here's my thought:

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
         // handle for the whole form (only once)
    } elseif (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
});
@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

@qiangxue the condition will work -- however we will have n validation calls doing the same validation since we have n events triggered in the loop.

Contributor

kartik-v commented Sep 8, 2014

@qiangxue the condition will work -- however we will have n validation calls doing the same validation since we have n events triggered in the loop.

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

PS: I had updated a cleaner code implementation example earlier (this is if we trigger the event outside the loop and pass all results in an array):

$('form').on('onValidationError', function(event, results) {
    if (results.length && results.attributes.length) {
         var attribs = results.attributes, j = 0, errors = [];
         for (var i = 0; i < attribs.length; i++) {
             if (attribs[i]['id'] == 'attribute1' || attribs[i]['id'] == 'attribute2' || attribs[i]['id'] == 'attribute3') {
                  errors[j] = attribs[i]['errors']; // attribute specific errors
                  j++;
             }
         }

         if (errors.length) {
             // do something
         }
    } else if (results.length) {
         var errors = results.errors; // all errors
        // still will fire only once
    }
});
Contributor

kartik-v commented Sep 8, 2014

PS: I had updated a cleaner code implementation example earlier (this is if we trigger the event outside the loop and pass all results in an array):

$('form').on('onValidationError', function(event, results) {
    if (results.length && results.attributes.length) {
         var attribs = results.attributes, j = 0, errors = [];
         for (var i = 0; i < attribs.length; i++) {
             if (attribs[i]['id'] == 'attribute1' || attribs[i]['id'] == 'attribute2' || attribs[i]['id'] == 'attribute3') {
                  errors[j] = attribs[i]['errors']; // attribute specific errors
                  j++;
             }
         }

         if (errors.length) {
             // do something
         }
    } else if (results.length) {
         var errors = results.errors; // all errors
        // still will fire only once
    }
});
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

I'm a bit lost... Are you proposing triggering an event for each failed input or not? In your code snippet, it seems so, but then what's the difference between yours and mine?

Member

qiangxue commented Sep 8, 2014

I'm a bit lost... Are you proposing triggering an event for each failed input or not? In your code snippet, it seems so, but then what's the difference between yours and mine?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

@qiangxue its correct and when triggered on input level it should be ok.

However when event is triggered on form - it will be triggered n times for n attributes if put within the loop.

So if we issue a code like this $('form').on('onValidationError') - it will be called n times. Its something like this in PHP:

for ($i=0; $i<=10; $i++) {
    execute_sql("UPDATE tbl SET id = 1 WHERE num = {$i};");
}

Though it will work - it executes the same repeated code multiple times.

Ideally speaking what we should do is within the loop not trigger the event for the form. Instead store/push the results into an array and not trigger the event until the loop closes for all attributes - and then at the end check if results array has errors - raise the event.

Contributor

kartik-v commented Sep 8, 2014

@qiangxue its correct and when triggered on input level it should be ok.

However when event is triggered on form - it will be triggered n times for n attributes if put within the loop.

So if we issue a code like this $('form').on('onValidationError') - it will be called n times. Its something like this in PHP:

for ($i=0; $i<=10; $i++) {
    execute_sql("UPDATE tbl SET id = 1 WHERE num = {$i};");
}

Though it will work - it executes the same repeated code multiple times.

Ideally speaking what we should do is within the loop not trigger the event for the form. Instead store/push the results into an array and not trigger the event until the loop closes for all attributes - and then at the end check if results array has errors - raise the event.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

Seems you just modified your code snippet. Now it's different from mine, and I think you are proposing to aggregate multiple input events into a single one to improve the performance, right? Do you think the performance impact is very big, considering the fact that both approaches still require similar amount of code execution within event handlers?

Member

qiangxue commented Sep 8, 2014

Seems you just modified your code snippet. Now it's different from mine, and I think you are proposing to aggregate multiple input events into a single one to improve the performance, right? Do you think the performance impact is very big, considering the fact that both approaches still require similar amount of code execution within event handlers?

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

I think you are proposing to aggregate multiple input events into a single one to improve the performance, right?

Yes when raising the event on the form element. For the input element it will still be individual events for each input.

Do you think the performance impact is very big, considering the fact that both approaches still require similar amount of code execution within event handlers?

The performance impact will be dependent on what the developer chooses to do with the event. For example, it can be significant if he is issuing a database or ajax call. The same code/call will be executed multiple times.

Contributor

kartik-v commented Sep 8, 2014

I think you are proposing to aggregate multiple input events into a single one to improve the performance, right?

Yes when raising the event on the form element. For the input element it will still be individual events for each input.

Do you think the performance impact is very big, considering the fact that both approaches still require similar amount of code execution within event handlers?

The performance impact will be dependent on what the developer chooses to do with the event. For example, it can be significant if he is issuing a database or ajax call. The same code/call will be executed multiple times.

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

How can that happen with the following event handler? If the developer only wants to issue a single ajax call, he should deal with attribute==undefined; if the developer wants to respond to a single attribute immediately, he should deal with attribute.id=='attribute1.

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
         // handle for the whole form (only once)
    } elseif (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
});
Member

qiangxue commented Sep 8, 2014

How can that happen with the following event handler? If the developer only wants to issue a single ajax call, he should deal with attribute==undefined; if the developer wants to respond to a single attribute immediately, he should deal with attribute.id=='attribute1.

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
         // handle for the whole form (only once)
    } elseif (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
});
@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

Will explain. Let's say he wants to trap all errors. The loop is from attribute1 to attribute10 for 10 attributes.

$('form').on('onValidationError', function(event, messages, attribute) {
    if (messages.length) {
        // any code here will be called 10 times
    } else if (attribute == undefined) {
         // handle for the whole form (only once)
    } else if (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
    // any code here will be called 10 times
});
Contributor

kartik-v commented Sep 8, 2014

Will explain. Let's say he wants to trap all errors. The loop is from attribute1 to attribute10 for 10 attributes.

$('form').on('onValidationError', function(event, messages, attribute) {
    if (messages.length) {
        // any code here will be called 10 times
    } else if (attribute == undefined) {
         // handle for the whole form (only once)
    } else if (attribute.id == 'attribute1') {
         // handle for a specific attribute (only once)
    }
    // any code here will be called 10 times
});
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

If he wants to trap all errors, he should place his code within if (attribute == undefined) {...} which is only called once.

Member

qiangxue commented Sep 8, 2014

If he wants to trap all errors, he should place his code within if (attribute == undefined) {...} which is only called once.

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

Yes that's correct. What I meant was one will not be able to trap all attribute-wise errors together in one shot. One can get all error messages but it will not be related to any attribute. For achieving this, it has to be through multiple if and else if statement blocks with this approach.

Just thinking loud... we are discussing about the form element only. Raising these events at input element as well can also help with another alternative.

Trapping for a specific attribute:

$('input').on('onValidationError', function(event, messages) {
   // do something
});

Trapping for multiple attributes:

$('form input').each(function() {
    var $input = $(this), id = $input.attr('id');
    if (id == 'attr1' || id == 'attr2' || id = 'attr3') {
        $input.on('onValidationError', function(event, messages) {
           // do something
        });
    }
});
Contributor

kartik-v commented Sep 8, 2014

Yes that's correct. What I meant was one will not be able to trap all attribute-wise errors together in one shot. One can get all error messages but it will not be related to any attribute. For achieving this, it has to be through multiple if and else if statement blocks with this approach.

Just thinking loud... we are discussing about the form element only. Raising these events at input element as well can also help with another alternative.

Trapping for a specific attribute:

$('input').on('onValidationError', function(event, messages) {
   // do something
});

Trapping for multiple attributes:

$('form input').each(function() {
    var $input = $(this), id = $input.attr('id');
    if (id == 'attr1' || id == 'attr2' || id = 'attr3') {
        $input.on('onValidationError', function(event, messages) {
           // do something
        });
    }
});
@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 8, 2014

Member

For achieving this, it has to be through multiple if and else if statement blocks with this approach.

It's similar to your approach. The difference is that your approach writes the loop inside a single event handler, while in my approach the loop is outside of the event handler.

Raising these events at input element as well can also help with another alternative.

It suffers from these issues:

  • Checkbox list or other similar input fields contain multiple inputs for a single field
  • Dynamically added input fields may not be handled well.
Member

qiangxue commented Sep 8, 2014

For achieving this, it has to be through multiple if and else if statement blocks with this approach.

It's similar to your approach. The difference is that your approach writes the loop inside a single event handler, while in my approach the loop is outside of the event handler.

Raising these events at input element as well can also help with another alternative.

It suffers from these issues:

  • Checkbox list or other similar input fields contain multiple inputs for a single field
  • Dynamically added input fields may not be handled well.
@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

The difference is that your approach writes the loop inside a single event handler, while in my approach the loop is outside of the event handler.

Understand the similarity in loop processing part. But when developer calls the event at runtime, is there a way for him to get attribute-wise messages data altogether in one shot? I mean he has to do something like this:

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
          // all messages for the form at once -- BUT...
         // can he get attribute-wise error messages for all attributes? If not, 
        // he has to do something like below assuming there are 3 attributes
    } else if (attribute.id == 'attribute1' || attribute.id == 'attribute2' || attribute.id == 'attribute3') {
         // do something ... however this code would fire up 3 times
    }
});
Contributor

kartik-v commented Sep 8, 2014

The difference is that your approach writes the loop inside a single event handler, while in my approach the loop is outside of the event handler.

Understand the similarity in loop processing part. But when developer calls the event at runtime, is there a way for him to get attribute-wise messages data altogether in one shot? I mean he has to do something like this:

$('form').on('onValidationError', function(event, messages, attribute) {
    if (attribute == undefined) {
          // all messages for the form at once -- BUT...
         // can he get attribute-wise error messages for all attributes? If not, 
        // he has to do something like below assuming there are 3 attributes
    } else if (attribute.id == 'attribute1' || attribute.id == 'attribute2' || attribute.id == 'attribute3') {
         // do something ... however this code would fire up 3 times
    }
});
@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 8, 2014

Contributor

I just updated the post on my code example earlier for the use-case to prevent multiple calls for a scenario like above.

Contributor

kartik-v commented Sep 8, 2014

I just updated the post on my code example earlier for the use-case to prevent multiple calls for a scenario like above.

@qiangxue qiangxue closed this in f50f840 Sep 10, 2014

@qiangxue

This comment has been minimized.

Show comment
Hide comment
@qiangxue

qiangxue Sep 10, 2014

Member

All done. I didn't add onValidationError because you can use afterValidate() and check if messages is empty or not to determine whether there is validation error.

Member

qiangxue commented Sep 10, 2014

All done. I didn't add onValidationError because you can use afterValidate() and check if messages is empty or not to determine whether there is validation error.

@kartik-v

This comment has been minimized.

Show comment
Hide comment
@kartik-v

kartik-v Sep 10, 2014

Contributor

This looks great. Will check in detail when I get time. I see the event names are configurable as well. Good.

Nice to have: The event names configuration array can be part of the yiiActiveForm plugin definition (e.g. the defaults array) instead of global - just for extensibility for multiple forms on same page.

Contributor

kartik-v commented Sep 10, 2014

This looks great. Will check in detail when I get time. I see the event names are configurable as well. Good.

Nice to have: The event names configuration array can be part of the yiiActiveForm plugin definition (e.g. the defaults array) instead of global - just for extensibility for multiple forms on same page.

cebe added a commit to cebe/yii2 that referenced this pull request Sep 10, 2014

Merge branch 'master' into Erik-r-2359-formatter-refactored
* master: (22 commits)
  Fixes yiisoft#4971: Fixed hardcoded table names in `viaTable` expression in model generator
  Fixed test break.
  Fixes yiisoft#4955: Replaced callbacks with events for `ActiveForm`
  Fix brackets
  Rename `\yii\web\User` component param for consistency
  Html::button() type is `button` by default
  Fix bug in Estonian translation
  Typo fixed inside \yii\rest\CreateAction
  Fixed test break.
  Fixed test case.
  note about validation rules order
  Return a fixtures cleanup in case of a failing test
  Update finnish translation
  skip fixture controller test on HHVM
  Make unit tests cleanup a DB after finish
  Fixes yiisoft#4945: Added `yii\test\ArrayFixture`
  added array fixture class
  minor doc adjustment [skip ci]
  Fixes yiisoft#4948. Thanks, @johan162
  Fixes yiisoft#4947
  ...

Conflicts:
	framework/UPGRADE.md
@thiagotalma

This comment has been minimized.

Show comment
Hide comment
@thiagotalma

thiagotalma Sep 10, 2014

Contributor

@kartik-v @qiangxue @cebe
Something was not cool. When the form is submitted by clicking the submit, the events are trigged twice.

Edit: $form.on('beforeSubmit') AND $form.on('submit') trigged twice.

Contributor

thiagotalma commented Sep 10, 2014

@kartik-v @qiangxue @cebe
Something was not cool. When the form is submitted by clicking the submit, the events are trigged twice.

Edit: $form.on('beforeSubmit') AND $form.on('submit') trigged twice.

@cebe

This comment has been minimized.

Show comment
Hide comment
@cebe

cebe Sep 11, 2014

Member

@thiagotalma please open a new issue!

Member

cebe commented Sep 11, 2014

@thiagotalma please open a new issue!

@J-jack-K

This comment has been minimized.

Show comment
Hide comment
@J-jack-K

J-jack-K Sep 11, 2014

Sorry how to use now "beforeSubmit"? I run the script but "return false" doesn't stop the submit.
Do you have some example?

J-jack-K commented Sep 11, 2014

Sorry how to use now "beforeSubmit"? I run the script but "return false" doesn't stop the submit.
Do you have some example?

@thiagotalma

This comment has been minimized.

Show comment
Hide comment
@thiagotalma
Contributor

thiagotalma commented Sep 11, 2014

@J-jack-K

This comment has been minimized.

Show comment
Hide comment
@J-jack-K

J-jack-K Sep 11, 2014

Thank thiagotalma but I haven't an ajax-form, I need to send a standard submit. So If I PreventDefault on submit how can I trigger the standard submit in BeforeSubmit?
Writing something like this obviously doesn't work:

$('#w0').on('beforeSubmit', function(event, messages, attribute) {
     if (($("#proforma-n_fatt").val()>0) || ($("#proforma-riemetti").prop("checked"))) {
            if (($("#proforma-n_fatt").val()>0) && ($("#proforma-riemetti").prop("checked"))) {
                $('#w0').submit();
                return true;
            } else
                return false;
    }
    $('#w0').submit(); 
    return true;
}).on('submit', function(e){ e.preventDefault();});

J-jack-K commented Sep 11, 2014

Thank thiagotalma but I haven't an ajax-form, I need to send a standard submit. So If I PreventDefault on submit how can I trigger the standard submit in BeforeSubmit?
Writing something like this obviously doesn't work:

$('#w0').on('beforeSubmit', function(event, messages, attribute) {
     if (($("#proforma-n_fatt").val()>0) || ($("#proforma-riemetti").prop("checked"))) {
            if (($("#proforma-n_fatt").val()>0) && ($("#proforma-riemetti").prop("checked"))) {
                $('#w0').submit();
                return true;
            } else
                return false;
    }
    $('#w0').submit(); 
    return true;
}).on('submit', function(e){ e.preventDefault();});
@thiagotalma

This comment has been minimized.

Show comment
Hide comment
@thiagotalma

thiagotalma Sep 11, 2014

Contributor

I knew this change would cause confusion ...

In your case, solved by setting one flag to control the submit:

var flag = false;
$('#w0').on('beforeSubmit', function(event, messages, attribute) {
     if (($("#proforma-n_fatt").val()>0) || ($("#proforma-riemetti").prop("checked"))) {
            if (($("#proforma-n_fatt").val()>0) && ($("#proforma-riemetti").prop("checked"))) {
                flag = true;
            }
}).on('submit', function(e){ 
if (!flag) {
 e.preventDefault();
}
});
Contributor

thiagotalma commented Sep 11, 2014

I knew this change would cause confusion ...

In your case, solved by setting one flag to control the submit:

var flag = false;
$('#w0').on('beforeSubmit', function(event, messages, attribute) {
     if (($("#proforma-n_fatt").val()>0) || ($("#proforma-riemetti").prop("checked"))) {
            if (($("#proforma-n_fatt").val()>0) && ($("#proforma-riemetti").prop("checked"))) {
                flag = true;
            }
}).on('submit', function(e){ 
if (!flag) {
 e.preventDefault();
}
});
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment