Open AlphaGit opened 8 years ago
Thanks for the library, it's very useful. Regarding this feature request, what do you mean by "consumer of the directive?" My initial thought is that the incorrect value could maybe be stored in the scope object, and the controller could do any handling of the value from there. Does that make sense?
Let me know, I'd be interested in working on this feature.
Hi @coreyc!
My initial thought was to raise an event every time that the validation took place, either successfully or unsuccessfully. Use cases would be for the consumer (controller that includes the template that uses the directive) to be notified of it and do something with it. For example, display a message that would help the user understand what the accepted values are. (See #39 for a quick discussion on it.)
My second thought was to pass on the value (the one coming from the input) so that the controller could perform any kind of logic and allow/prevent validation. Use cases go to any validation logic beyond regexes. However, this means that the controller needs to be executed, and this means waiting for the $$digest cycle to occur, before the directive can override the value in the input.
Right now the directive works by being very close to the DOM, intercepting the events and updating the input value as soon as possible, so that the next $$digest cycle will sync the right model values to the Angular controllers. Deferring it could bring us trouble with keeping the model in sync, so that's something I wanted to avoid.
However, let me know what your thoughts are. I am definitely not opposed to providing controllers/templates more flexibility over the internal workings, but at the same time I would like to avoid the complexity of working around Angular cycle's without a clean approach.
Thanks!
Been really busy with work recently, but haven't forgotten about this. I'll fork and test out some ideas then let you know what I come up with.
@coreyc Thanks for the heads up!
Ok, I played around with some ideas that were easiest to demonstrate in a fiddle: https://jsfiddle.net/fecdphex/3/
Allow consumers of the directive to be notified when a particular value passed or did not pass the validation.
If possible, allow them to see the entered value before it is replaced with the last good known instance.
These first two are accomplished pretty simply by emitting an event from the revertToPreviousValue() function, that the controller can then catch using scope.$on. There are pros and cons to this. On the one hand, users of this directive will have to know the name of the event to look for in the $on function in order to add it to the controller, but on the other hand they aren't forced to doing anything with this event. The emitted event isn't doing any harm by just being there.
If possible, allow them to set the value to be set in place of the last good known instance.
I think I've worked out a way to do this without the model being out of sync. I've stuck it in the revertToPreviousValue() function for simplicity's sake for now. One issue I'm still working through is the following (demonstrated in the fiddle):
I'm not sure what the use case would be for this kind of scenario, but if there was one I guess there are two options:
One thing I don't like about this, but wasn't able to figure out a way around, is that now the controller seems to be more 'coupled' to the directive in that it has to know more about how the directive works in order to implement override logic. This may just be unavoidable, but I wanted to call it out. Also, as I was working through this, I wondered if allowing a consumer of the directive this much control is the right approach. I see the use case, but wonder if extending the directive in this way might be expanding beyond its intent? In other words, if the controller wants to correct/alter the invalid value, couldn't it setup some watchers and handle the evaluation and correction on its own?
Phew, that's a lot of text. Let me know if this makes sense!
Hi @coreyc -- thanks for all the work and yes, your explanation does make sense. Thanks for being so thoughtful and providing such a thorough analysis. Let me address it point by point.
These first two are accomplished pretty simply by emitting an event from the revertToPreviousValue() function, that the controller can then catch using scope.$on. There are pros and cons to this. On the one hand, users of this directive will have to know the name of the event to look for in the $on function in order to add it to the controller, but on the other hand they aren't forced to doing anything with this event. The emitted event isn't doing any harm by just being there.
I agree with this point of view. I don't see any problem with events being part of the directive API, although I personally lean towards using attributes and bind to functions. This can also be optional and feels more declarative, in the same fashion that most directives would feel. Also, I personally don't like polluting the scope but I also think there might be use cases where it's the right choice.
However, in this case I saw that you were using events as a two-way communication path. I think this can get tricky way too fast since we're relaying at some point on the order of evaluation and how likely is it that Angular will process those events before doing something else with the model binding. Going back to my function-binding approach, I think that could be overcome by making the result of the function being part of the API too. For example:
<input ng-pattern-restrict="^[0-9]*$" ng-pattern-restrict-on-validation-failed="customValidation" />
// in controller
$scope.customValidation = function(val) {
if (!val) {
$scope.error = "A value is required!"; // for demonstration purposes of a "real" use case
return; // returning falsy: no change to the value
}
if (+val > 20) {
$scope.error = "Maximum is 20"; // for demonstration purposes of a "real" use case
return "20"; // returning a value: value to use for replacement
}
}
I'm not saying it has to be exactly like this, but something of the sort.
This also populates on the scope, but it's the controller that needs to do it, only if needed -- which is the usual approach for these type of directive-controller bindings.
One issue I'm still working through is the following (demonstrated in the fiddle): (...)
Yes, I can't think of a way to get users out of such scenario either. But it seems that it will be more related to the actual usage of the directive by the programmer than by the logic of the directive itself. I don't think we should over-complicate our design to prevent developers form shooting themselves in the foot, but if there's an elegant way for us to do so, it is most welcome.
Also, as I was working through this, I wondered if allowing a consumer of the directive this much control is the right approach. I see the use case, but wonder if extending the directive in this way might be expanding beyond its intent? In other words, if the controller wants to correct/alter the invalid value, couldn't it setup some watchers and handle the evaluation and correction on its own?
This is a deep and thoughtful question and I'm really glad you brought it up.
While I do have concerns that the community may come up with requests that go beyond the original scope, I don't think this is the case. The original intention of the directive (which can mutate, definitely) was to prevent users from typing characters that would make a validation go wrong, and hiding inside of a directive all that "ugliness" required to make that typing-not-working transparent to an AngularJS user. That's why we work so close to the DOM here and get to fight browser quirks.
However, what should happen when a user effectively invalidates goes for the most sensitive option: go back to the last good known scenario. I think this is, on one hand, a decision that could be taken out of the directive (while keeping it as a sane default) and, on the other hand, a decision that is not always the best case.
For example, lets say that your input text, accepting numbers only has "12345" and in a single operation you paste "54232a". Maybe the most sensible approach here would be to leave "54232". Maybe it would be to go back to "12345". Maybe reset to zero? I think the answer is most likely related to the business use case here.
That's why I saw this request as a legitimate improvement.
Aside from the directive notifying "this happened", which is definitely useful, like in @irlnathan's example. (@irlnathan, you're most welcome to suggest ideas for this feature too.)
In other words, if the controller wants to correct/alter the invalid value, couldn't it setup some watchers and handle the evaluation and correction on its own?
Absolutely. But we do want to save them the tricky part of handling events to make it transparent to a user that happens to be typing/texting.
PS: Don't feel bad at all for sending walls of text, you can see I'm guilty on that one too, lol. Honestly, I'll read it all and pay attention. I really appreciate explanations and reasons to understand what the best approach for something might be. :)
However, in this case I saw that you were using events as a two-way communication path. I think this can get tricky way too fast since we're relaying at some point on the order of evaluation and how likely is it that Angular will process those events before doing something else with the model binding. Going back to my function-binding approach, I think that could be overcome by making the result of the function being part of the API too. For example:
Ah yes, good catch. Assuming we'd want to use isolate scope here, something like:
// in directive
scope: {
ngPatternRestrictOnValidationFailed: '&'
}
If we do go with function-binding, won't the invalid value still need to be populated on $scope in order to be accessed in the controller function? I'll admit, passing data around between directives and controllers trips me up sometimes.
Regarding my comment on the intention of this directive, I think your response definitely makes sense. What I was getting at was wanting to avoid turning this directive back into Angular's built-in ng-pattern.
Here's a very simple example created from the code you suggested:
app.controller('MainCtrl', function($scope) {
$scope.showMessage = function() {
alert('Hi from the controller!');
};
}).directive('myDirective', function() {
return {
template: '<button ng-click="innerClick()">Click me!</button>',
scope: {
innerClick: '&myDirectiveButtonClicked'
}
};
});
<my-directive my-directive-button-clicked="showMessage()" />
As you can see, the directive can invoke a particular function at some point. If we're into the link function, we can do something like this:
app.controller('MainCtrl', function($scope) {
$scope.showMessage = function() {
alert('Hi from the controller!');
};
}).directive('myDirective', function() {
return {
template: '<button>Click me!</button>',
scope: {
innerClick: '&myDirectiveButtonClicked'
},
link: function(scope, element, attr) {
element.on('click', function() {
scope.innerClick();
});
}
};
});
So, basically, we can use an internal scope which is linked to the external one through the use of a binding expression. I'm also guessing we can make them optional and test for their presence, or bind them to no-op functions if they were not set (in which case I'd expect them to be undefined or null).
Our invocation can pretty much also wait for the return value and operate on it.
Allow consumers of the directive to be notified when a particular value passed or did not pass the validation.
If possible, allow them to see the entered value before it is replaced with the last good known instance.
If possible, allow them to set the value to be set in place of the last good known instance.