On 07/06/2011 08:12 PM, Endi Sukma Dewata wrote:
On 7/6/2011 4:32 PM, Adam Young wrote:


Some issues:

1. The check_required() is only called in blur events. It's not called on Add/Update.

Fixed.  Looks like this works even for checkboxes.


To test, open user's adder dialog, don't enter anything, just click Add. The server will return an error (i.e. check_required() not called).

Another test, edit an existing user, empty the first name, click somewhere else, an error will appear because it loses focus. Then click Update, the server will return an error (i.e. check_required() not called).

2. In IPA.entity_select_widget the check_required() is only called if the widget is editable.

To test, open IPA Server -> Configuration, set the Default user group to empty, then click somewhere else. There's no validation error.
Fixed, but I don't think that there is currently a testable case for this, as many things don't have required set.

3. Also in IPA.entity_select_widget the check_required() is only called from the text input's blur event, not from the drop down list. This leads to strange behavior:

Open the hosts' adder dialog, click the drop down list, the validation error will appear before the user has a chance to select a value.
Again fixed ,but not sure it is verifiable. host and service add don't seem to have metadata for required.


4. For consistency, the multivalued_text and textarea widgets can be modified to call the create_error_link() to create the error_link element.
done

5. There's a jslint warning.
fixed.

From 49bfa7f348133f3fffcbb23f8b858a3f4d327361 Mon Sep 17 00:00:00 2001
From: Adam Young <[email protected]>
Date: Wed, 6 Jul 2011 15:43:50 -0400
Subject: [PATCH] check required on blur

previsouly was checked on key down, but that does the check too soon.
works for entity_select widget, too
Checks upon form submission

https://fedorahosted.org/freeipa/ticket/1437
---
 install/ui/add.js    |   11 ++++++-
 install/ui/widget.js |   76 ++++++++++++++++++++++++++++++++-----------------
 2 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/install/ui/add.js b/install/ui/add.js
index 50b6124c094c60f18c32897da603331db8c6a161..0a414b74b181e8d20fd80594a00f93b25c688f6e 100644
--- a/install/ui/add.js
+++ b/install/ui/add.js
@@ -118,11 +118,14 @@ IPA.add_dialog = function (spec) {
         for (var i=0; i<fields.length; i++) {
             fields[i].validate();
         }
-
+        var required_fields_filled = true;
         for (i=0; i<fields.length; i++) {
             field = fields[i];
             if (!field.valid) return;
 
+            required_fields_filled = field.check_required() &&
+                required_fields_filled;
+
             value = record[field.name];
             if (!value) continue;
 
@@ -141,6 +144,8 @@ IPA.add_dialog = function (spec) {
             for (var k=0; k<section_fields.length; k++) {
                 field = section_fields[k];
                 if (!field.valid) return;
+                required_fields_filled = field.check_required()  &&
+                    required_fields_filled;
 
                 value = record[field.name];
                 if (!value) continue;
@@ -155,7 +160,9 @@ IPA.add_dialog = function (spec) {
 
         //alert(JSON.stringify(command.to_json()));
 
-        command.execute();
+        if (required_fields_filled){
+            command.execute();
+        }
     };
 
     that.add_dialog_init = that.init;
diff --git a/install/ui/widget.js b/install/ui/widget.js
index cd3a5c60e2153b25c0fce58ebaf94cf3f51f1ffe..6c37d945dcdae354bad4e4712c341c4b341aea53 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -107,27 +107,39 @@ IPA.widget = function(spec) {
         }
 
     }
+    that.create_error_link = function(container){
+        container.append(' ');
+
+        $('<span/>', {
+            name: 'error_link',
+            html: IPA.messages.widget.validation.error,
+            'class': 'ui-state-error ui-corner-all',
+            style: 'display:none'
+        }).appendTo(container);
+    };
+
+    that.check_required = function(){
+        var values = that.save();
+        if (!values || !values.length || values[0] === '' ) {
+            if (that.param_info &&
+                that.param_info.required &&
+                !that.optional) {
+                that.valid = false;
+                that.show_error(IPA.messages.widget.validation.required);
+                return false;
+            }
+        }
+        return true;
+    };
 
     /*returns true and clears the error message if the field value  passes
       the validation pattern.  If the field value does not pass validation,
       displays the error message and returns false. */
     that.validate = function() {
-
         that.hide_error();
-
         that.valid = true;
 
         var values = that.save();
-        if (!values || !values.length) {
-            if (that.param_info &&
-                that.param_info.required &&
-                !that.optional) {
-                that.valid = false;
-                that.show_error(IPA.messages.widget.validation.required);
-            }
-            return;
-        }
-
         var value = values[0];
         if (!value) {
             return;
@@ -371,6 +383,7 @@ IPA.text_widget = function(spec) {
         IPA.select_range(that.input, start, end);
     };
 
+
     that.create = function(container) {
 
         $('<label/>', {
@@ -391,14 +404,7 @@ IPA.text_widget = function(spec) {
             that.create_undo(container);
         }
 
-        container.append(' ');
-
-        $('<span/>', {
-            name: 'error_link',
-            html: IPA.messages.widget.validation.error,
-            'class': 'ui-state-error ui-corner-all',
-            style: 'display:none'
-        }).appendTo(container);
+        that.create_error_link(container);
     };
 
     that.setup = function(container) {
@@ -411,6 +417,10 @@ IPA.text_widget = function(spec) {
             that.validate();
         });
 
+        input.blur(function(){
+            that.check_required();
+        });
+
         var undo = that.get_undo();
         undo.click(function() {
             that.reset();
@@ -1079,12 +1089,8 @@ IPA.textarea_widget = function (spec) {
             that.create_undo(container);
         }
 
-        $("<span/>",{
-            name:'error_link',
-            html: IPA.messages.widget.validation.error,
-            "class":"ui-state-error ui-corner-all",
-            style:"display:none"
-        }).appendTo(container);
+        that.create_error_link(container);
+
     };
 
     that.setup = function(container) {
@@ -1098,6 +1104,10 @@ IPA.textarea_widget = function (spec) {
 
         });
 
+        input.blur(function(){
+            that.check_required();
+        });
+
         var undo = that.get_undo();
         undo.click(function() {
             that.reset();
@@ -1649,21 +1659,33 @@ IPA.entity_select_widget = function(spec) {
         if (editable){
             that.edit_box = $('<input />',{
                 type: 'text',
-                title: that.tooltip
+                title: that.tooltip,
+                blur:function(){
+                    that.check_required();
+                },
+                keyup:function(){
+                    that.hide_error();
+                }
             });
 
             $('<div style:"display=block;" />').
                 append(that.edit_box).
                 appendTo(container);
+
+            that.create_error_link(container);
+
         }
 
         that.entity_select = $('<select/>', {
             id: that.name + '-entity-select',
             change: function(){
+                that.hide_error();
                 if (editable){
                     that.edit_box.val(
                         $('option:selected', that.entity_select).val());
                     IPA.select_range(that.edit_box,0,0);
+                }else{
+                    that.check_required();
                 }
                 that.set_dirty(that.test_dirty());
             }
-- 
1.7.5.2

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to