feat #964 Show error tooltip whenever there is an error #964

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@smadapathi
Contributor

This pull request allows error tooltip to appear as soon as there is an error while typing. For this we have introduced validation event 'onError'.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/CfgBeans.js
@@ -246,6 +246,10 @@ Aria.beanDefinitions({
$type : "json:Boolean",
$description : "Whether validation on input widgets is automatically called by default on blur."
},
+ "directOnErrorValidation" : {
+ $type : "json:Boolean",
+ $description : "Whether validation on input widgets is automatically called by default on keydown."
+ },
@divdavem
divdavem Feb 11, 2014 Member

This property is not used in the widget, and is not mentioned in the spec. Please remove it.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/environment/WidgetSettingsCfgBeans.js
@@ -45,6 +45,11 @@ Aria.beanDefinitions({
$description : "Whether validation on input widgets is automatically called by default on blur.",
$default : true
},
+ "directOnErrorValidation" : {
+ $type : "json:Boolean",
+ $description : "Whether validation on input widgets is automatically called by default on keydown.",
+ $default : false
+ },
@divdavem
divdavem Feb 11, 2014 Member

This property is not used in the widget, and is not mentioned in the spec. Please remove it.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/form/Input.js
@@ -364,7 +364,11 @@ Aria.classDefinition({
// environment
cfg.directOnBlurValidation = aria.widgets.environment.WidgetSettings.getWidgetSettings().directOnBlurValidation;
}
-
+ if (cfg.directOnErrorValidation == null) {
+ // the default value for directOnErrorValidation comes from the
+ // environment
+ cfg.directOnErrorValidation = aria.widgets.environment.WidgetSettings.getWidgetSettings().directOnErrorValidation;
+ }
@divdavem
divdavem Feb 11, 2014 Member

directOnErrorValidation is not used in the widget, and is not mentioned in the spec. Please remove those lines.

@divdavem divdavem commented on the diff Feb 11, 2014
src/aria/widgets/form/MultiAutoComplete.js
event.preventDefault();
var report = this.controller.checkText(inputFieldValue, false);
this._reactToControllerReport(report);
this.setHelpText(false);
+ inputField.focus();
@divdavem
divdavem Feb 11, 2014 Member

Why is this line needed?

@smadapathi
smadapathi Feb 12, 2014 Contributor

This is to keep focus back to input (for a new suggestion) when user edits a suggestion, types freeText and press tab key.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/controllers/AutoCompleteController.js
@@ -372,6 +372,9 @@
} else {
if (!this.freeText && suggestionsAvailable && !hasSuggestions) {
report.ok = false;
+ var errorReport = this.checkText(nextValue);
+ this.widgetObj.changeProperty("formatErrorMessages", errorReport.errorMessages);
+ errorReport.$dispose();
@divdavem
divdavem Feb 11, 2014 Member

The controller is not supposed to have a reference to the widget object. Any communication from the controller to the widget is supposed to be done through the report object.
In this method, we are already in the process of defining a report to be sent to the widget.
So, let's simply use the report which is already defined in order to pass the error messages to the widget.

So, I would replace those 3 lines with the following line:

report.errorMessages.push(this.res.errors["40020_WIDGET_AUTOCOMPLETE_VALIDATION"]);
@divdavem divdavem commented on an outdated diff Feb 11, 2014
...ia/widgets/controllers/MultiAutoCompleteController.js
@@ -296,6 +296,9 @@
} else {
if (!this.freeText && suggestionsAvailable && !hasSuggestions) {
report.ok = false;
+ var errorReport = this.checkText(nextValue);
+ this.widgetObj.changeProperty("formatErrorMessages", errorReport.errorMessages);
+ errorReport.$dispose();
@divdavem
divdavem Feb 11, 2014 Member

Same remark as before:
The controller is not supposed to have a reference to the widget object. Any communication from the controller to the widget is supposed to be done through the report object.
In this method, we are already in the process of defining a report to be sent to the widget.
So, let's simply use the report which is already defined in order to pass the error messages to the widget.

So, I would replace those 3 lines with the following line:

report.errorMessages.push(this.res.errors["40020_WIDGET_AUTOCOMPLETE_VALIDATION"]);
@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/form/AutoComplete.js
@@ -59,6 +59,7 @@ Aria.classDefinition({
controllerInstance.maxlength = cfg.maxlength;
controllerInstance.expandButton = cfg.expandButton;
controllerInstance.selectionKeys = cfg.selectionKeys;
+ controllerInstance.widgetObj = this;
@divdavem
divdavem Feb 11, 2014 Member

The controller is not supposed to have a reference to the widget object. Any communication from the controller to the widget is supposed to be done through a report object.
Please remove this line.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/form/MultiAutoComplete.js
@@ -37,6 +37,7 @@ Aria.classDefinition({
this._hideIconNames = ["dropdown"];
controller.maxOptions = cfg.maxOptions;
+ controller.widgetObj = this;
@divdavem
divdavem Feb 11, 2014 Member

The controller is not supposed to have a reference to the widget object. Any communication from the controller to the widget is supposed to be done through a report object.
Please remove this line.

@divdavem divdavem commented on an outdated diff Feb 11, 2014
src/aria/widgets/form/TextInput.js
@@ -711,6 +711,14 @@ Aria.classDefinition({
|| propertyName === 'errorMessages') {
this._cfg[propertyName] = newValue;
this._reactToChange();
+ var cfg = this._cfg;
+ if (cfg && cfg.validationEvent === 'onError') {
@divdavem
divdavem Feb 11, 2014 Member

According to the specifications, when validationEvent is set to "onError" the error tooltip should only appear when the widget has the focus.
So I would replace the previous line with:

if (cfg && cfg.validationEvent === 'onError' && (this._keepFocus || this._hasFocus)) {
@divdavem
Member

For my suggestions to work correctly, you will also need to move the code which sets formatErrorMessages from the checkValue method to the _reactToControllerReport method in TextInput.js.

To do that you could remove the following 3 lines from checkValue here (lines 439-441):

if (this._cfg.directOnBlurValidation) {
    this.changeProperty("formatErrorMessages", report.errorMessages);
}

and add the following 3 lines in _reactToControllerReport, just before the if (!delayedValidation) condition (line 515):

if (report.errorMessages.length && this._cfg.directOnBlurValidation) {
    this.changeProperty("formatErrorMessages", report.errorMessages);
}
@smadapathi smadapathi pushed a commit to smadapathi/ariatemplates that referenced this pull request Feb 12, 2014
smadapathi feat #964 Changes for Error tooltip on keydown 33b39a8
@smadapathi smadapathi pushed a commit to smadapathi/ariatemplates that referenced this pull request Feb 12, 2014
smadapathi feat #964 Changes for Error tooltip on keydown 9b9bea0
@smadapathi smadapathi pushed a commit that referenced this pull request Feb 12, 2014
@divdavem smadapathi + divdavem feat #964 Changes for Error tooltip on keydown
close #964
41d50bf
@smadapathi smadapathi pushed a commit that closed this pull request Feb 12, 2014
@divdavem smadapathi + divdavem feat #964 Changes for Error tooltip on keydown
close #964
41d50bf
@flongo flongo added this to the 1.4.16 milestone Feb 13, 2014
@divdavem divdavem was assigned by flongo Feb 13, 2014
@smadapathi smadapathi pushed a commit to smadapathi/ariatemplates that referenced this pull request Feb 18, 2014
smadapathi feat #964 Changes for Error tooltip on keydown 05d7f53
@smadapathi smadapathi pushed a commit to smadapathi/ariatemplates that referenced this pull request Feb 21, 2014
smadapathi feat #964 Changes for Error tooltip on keydown d8a07c6
@carlo-mr carlo-mr added a commit to carlo-mr/ariatemplates that referenced this pull request Mar 3, 2014
@carlo-mr smadapathi + carlo-mr feat #964 Changes for Error tooltip on keydown
close #964
fdd1717
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment