Hi Wanja,
I think your email contains very valuable input.
Personally, I agree that the framework would be more flexible if the validation rule did not make assumptions on WHEN to validate and WHAT to do when the validation fails. A couple of points to discuss:
1. I'm not sure what exactly you mean with 'highlight'. Is that an error or warning marker shown next to the widget? Is that some characters in the text widget being highlighted? The first should be possible; the second is currently not possible, because we do not have a way to know which characters in the input are invalid (this is information that each rule would have to provide).
2. I believe the original motivation for having different validation times (on_edit vs on_update) was that some validations can be too expensive to be performed on every keystroke. Nevertheless implementation would be simpler if we only supported one validation time (for example on edit). Currently the rules seem pretty straightforward in terms of "expensiveness" but I'm sure there are more complex examples in the wild.
3. Assuming that we want different validation times: Maybe the validation time should be passed as a constructor argument instead of being set via inheritance (as currently). Having to subclass a rule to change validation time seems a bit awkward. The constructor argument also forces the developer to make a choice WHEN to validate.
4. Regarding the rule's constructor: one idea would be to use the '|' operator to be more swt-like: new MinLength(3, IRidget.Hints.TRIGGER_DIALOG | IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT); However I'm open to either style and the '|' operator places some constraints on the values used for the constants.
Food for thought.
Regards,
Elias.
First of all, sorry if my mail breaks some threading in your mail client,
I have just subscribed to this list and need to copy some of the texts from the archive.
> From: "Volanakis, Elias":
> [case a+b]
> user types in widget
> -> on edit validation -> update ridget text
> -> focus lost / CR -> on update validation -> update model
>
> [case c]
> ridget.setText()
> -> on edit and on update validation
> if !ok -> throw runtime exception; widget, ridget, model unchanged
> if ok -> update widget and model; widget, ridget, model have same
> value
>
> [case d]
> model.setValue("xxx"); ridget.updateFromModel()
> -> on edit and on update validation
> if !ok -> throw runtime exception; widget and ridget unchanged
> if ok -> update widget and ridget; widget, ridget, model have same value
I second that.
If the application developer sets a value, he should be aware of what he sets, so it's okay to raise an exception. He could check the rules on his own before
setting the new value, if he really needed to.
This is also how partial editing could be realized, when the rules were strict.
1. write user input to buffer
2. validate buffer
3. if buffer valid write buffer to model
I do fancy rules which are strict.
In my opinion rules have to check whether input is valid or not with nothing in between. Whether or not or even when invalid input is accepted, should be up to the class using the rule, not the rule itself.
If we had a textfield which wants at least 3 letters to be valid,
popping up a dialog and blocking the input on errors would me a user's nightmare: as soon as he liked to correct the last character, he needed to write the new character and delete the previous one, instead of dleting the last one and entering the new one. Of course, everything that was acceptable here would be highlighting the error, not showing a dialog.
Anyway, the application developer may still like to see a dialog when input fails on CR/lost focus (where I usually think a dialog is the wort possible way to indicate errors, as it interrupts the workflow of the user forcing him to click a some button somewhere on the screen before he can go on and fix the wrong input, but that's an entirely different issue).
So it should be possible for the application developer to configure his ridget the way, that it (optinally) highlights while typing and (optionally) shows a dialog on CR/focus lost. This is however ridget logic, not rule logic.
Because of that, I also dislike the idea that the rules return a fixed value which tells when the rules likes to be evaluated, lke they do today (see validationTime).
I would prefer if each rule, by default, was always evaluated (=on input) and the developer could set the validation time to another value if necessary.
Maybe some more generic parameters to the rule would be a nice idea.
Imagine this (using some pseudocode here):
-- user code:
ridget.addRule(new MinLength(3, IRidget.Hints.TRIGGER_DIALOG, IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT));
ridget.addRule(new MaxLength(12, IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT));
-- framwork code:
class AbstractValidationRule implements IValidationRule{
AbstractValidationRule(Object... hints){..}
//...
}
class MinLength{
private final int len;
MinLength(int len, Object... hints){
super(hintes);
this.len=len;
}
//...
}
Where the ridget does something like:
//... on input:
if(updateHighlightsOnInput){
for(IValidationRule rule : rules){
jointStatus.join(rule.validate(input)); //also joins hints
}
if(!jointStatus.isOk()){
if(jointStatus.containsHint(IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT)){
setErrorMarker(jointStatus);
}else if(jointStatus.containsHint(IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT)){
setWarningMarker(jointStatus);
removeErrorMarker();
updateModel(...);//update anyway, this is just a warning.
}
}else {//everything is okkay
removeErrorAndWarningMarkers();
updateModel(...);
}
}
//... on update:
for(IValidationRule rule : rules){
jointStatus.join(rule.validate(input)); //also joins hints
}
if(!jointStatus.isOk()){
if( jointStatus.containsHint(IRidget.Hints.TRIGGER_DIALOG)){
triggerDialog(jointStatus);
}
if(jointStatus.containsHint(IRidget.Hints.TRIGGER_ERROR_HIGHLIGHT)){
setErrorMarker(jointStatus);
}else if(jointStatus.containsHint(IRidget.Hints.TRIGGER_WARNING_HIGHLIGHT)){
setWarningMarker(jointStatus);
removeErrorMarker();
updateModel(...);//update anyway, this is just a warning.
}
}else{ //everything is okkay
removeErrorAndWarningMarkers();
updateModel(...);
}
//...
This way the user could configure the ridget to
1. whether or not update any highlight on input (just on update)
2. show a dialog for severe errors and a highlight for minor ones
3. even highlight warnings, which do not block writing to the model, like
"texts of this length may be unusable with older clients" or
"it is not recommended to use an abolute path, try '$PROJECT/my/subdirectory/'".
What do you think?
Regards,
-Wanja-
