On 10/31/2011 11:38 PM, Endi Sukma Dewata wrote:
On 10/27/2011 4:57 AM, Petr Vobornik wrote:
https://fedorahosted.org/freeipa/ticket/1459

Some issues:

1. The new clear() method is called during refresh(), so the facet with
the old data is still shown for a brief moment before it's cleared.

The clear() should be called before show(). However, if the pkey/filter
is unchanged (check using needs_update()) we just need to show() the
facet, no need to clear() and refresh() again. The above logic needs to
be fixed.

Changed.


The we will need to override the needs_update() for search and
association facets because the default one always returns true.

Done

2. The following code in association.js and search.js will call clear()
if there's no old pkey/filter, is this intentional? No old pkey/filter
means the page is just loaded, so it probably doesn't need clearing.

if (!old_pkey || old_pkey[0] !== pkey[0]) {
that.clear();
}

if (!old_filter || old_filter[0] !== filter[0]) {
that.clear();
}
It seems unnecessary. But probably it was intentional (don't remember) - IDRC if there is a widget - maybe keytab or certificate status, which has some default state worth cleaning. Anyway in current implementation this logic is part of need_update and it is a must. IMHO we should avoid implementing special need_cleaning method. Cleaning at first display doesn't do any harm and it is one less method to maintain in couple classes.


3) Fixed bad implementation of clear method in radio_widget.

4) Changed direct/indirect radio names in association facets - radios form different facets were interfering.

5) Added ID generator, using in radio_widget, same reason as #4.

6) Added clearing of header in details facet and association facets - refreshes of member counts were confusing.

7) Removed setting that.pkey in create method in details, association facet (it broke need_update, didn't found purpose).

8) Maybe we should add a refresh button to search facet. It doesn't reflect concurrent usage. Refresh by 2 changes of filter doesn't seem nice.


--
Petr Vobornik
From d99d152ea71f89459b4cdb2b60690cc771e1b8fc Mon Sep 17 00:00:00 2001
From: Petr Vobornik <pvobo...@redhat.com>
Date: Mon, 24 Oct 2011 14:53:29 +0200
Subject: [PATCH] Page is cleared before it is visible

https://fedorahosted.org/freeipa/ticket/1459

Changes:
 * added clear method to widgets, section, search, details, association facets
 * clear and refresh method in facet are called only if key/filter was changed
 * added id generator for widgets
---
 install/ui/association.js |   21 ++++++++---
 install/ui/certificate.js |    9 +++++
 install/ui/details.js     |   21 ++++++++++--
 install/ui/entity.js      |   19 +++++++++--
 install/ui/host.js        |   12 +++++++
 install/ui/search.js      |   18 ++++++++--
 install/ui/service.js     |    5 +++
 install/ui/user.js        |    5 +++
 install/ui/widget.js      |   80 +++++++++++++++++++++++++++++++++++++++------
 9 files changed, 164 insertions(+), 26 deletions(-)

diff --git a/install/ui/association.js b/install/ui/association.js
index d3b66132d5043b0dfe60b8847896e9f27f676059..d3d6b124b431414ff04fad05b16dbb972b38c2b7 100644
--- a/install/ui/association.js
+++ b/install/ui/association.js
@@ -871,8 +871,6 @@ IPA.association_facet = function (spec) {
 
         that.facet_create_header(container);
 
-        that.pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-
         if (!that.read_only) {
             that.remove_button = IPA.action_button({
                 name: 'remove',
@@ -908,12 +906,13 @@ IPA.association_facet = function (spec) {
             span.append(IPA.messages.association.show_results);
             span.append(' ');
 
-            var direct_id = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-direct-radio';
+            var name = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-type-radio';
+            var direct_id = name + '-direct';
 
             that.direct_radio = $('<input/>', {
                 id: direct_id,
                 type: 'radio',
-                name: 'type',
+                name: name,
                 value: 'direct',
                 click: function() {
                     that.association_type = $(this).val();
@@ -929,12 +928,12 @@ IPA.association_facet = function (spec) {
 
             span.append(' ');
 
-            var indirect_id = that.entity.name+'-'+that.attribute_member+'-'+that.other_entity+'-indirect-radio';
+            var indirect_id = name + '-indirect';
 
             that.indirect_radio = $('<input/>', {
                 id: indirect_id,
                 type: 'radio',
-                name: 'type',
+                name: name,
                 value: 'indirect',
                 click: function() {
                     that.association_type = $(this).val();
@@ -1201,6 +1200,16 @@ IPA.association_facet = function (spec) {
         command.execute();
     };
 
+    that.clear = function() {
+        that.header.clear();
+        that.table.clear();
+    };
+
+    that.needs_update = function() {
+        var pkey = IPA.nav.get_state(that.entity.name+'-pkey');
+        return that.pkey !== pkey;
+    };
+
     /*initialization*/
     var adder_columns = spec.adder_columns || [];
     for (var i=0; i<adder_columns.length; i++) {
diff --git a/install/ui/certificate.js b/install/ui/certificate.js
index 5439c4920e1ace3e33fdaf8e0536f01990a1669d..d3411d05f12ae5579cefa15fd8482b0563960af9 100755
--- a/install/ui/certificate.js
+++ b/install/ui/certificate.js
@@ -725,6 +725,15 @@ IPA.cert.status_widget = function(spec) {
         }
     };
 
+    that.clear = function() {
+        that.status_valid.css('display', 'none');
+        that.status_missing.css('display', 'none');
+        that.status_revoked.css('display', 'none');
+        that.revoke_button.css('display', 'none');
+        that.restore_button.css('display', 'none');
+        that.revocation_reason.text('');
+    };
+
     function set_status(status, revocation_reason) {
         that.status_valid.css('display', status == IPA.cert.CERTIFICATE_STATUS_VALID ? 'inline' : 'none');
         that.status_missing.css('display', status == IPA.cert.CERTIFICATE_STATUS_MISSING ? 'inline' : 'none');
diff --git a/install/ui/details.js b/install/ui/details.js
index 98f48d0f9f02f323fd1b1ef206f2ece198d87c90..15056204f72ef2095862c2c35d24cd47fbc819b3 100644
--- a/install/ui/details.js
+++ b/install/ui/details.js
@@ -199,6 +199,14 @@ IPA.details_section = function(spec) {
         }
     };
 
+    that.clear = function() {
+        var fields = that.fields.values;
+
+        for (var i=0; i< fields.length; i++) {
+            fields[i].clear();
+        }
+    };
+
     init();
 
     // methods that should be invoked by subclasses
@@ -409,8 +417,6 @@ IPA.details_facet = function(spec) {
 
         that.facet_create_header(container);
 
-        that.pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-
         that.create_controls();
 
         that.expand_button = IPA.action_button({
@@ -523,7 +529,7 @@ IPA.details_facet = function(spec) {
 
     that.needs_update = function() {
         var pkey = IPA.nav.get_state(that.entity.name+'-pkey');
-        return pkey != that.pkey;
+        return pkey !== that.pkey;
     };
 
     that.section_dirty_changed = function(dirty) {
@@ -720,6 +726,15 @@ IPA.details_facet = function(spec) {
         command.execute();
     };
 
+    that.clear = function() {
+        that.header.clear();
+        var sections = that.sections.values;
+
+        for (var i=0; i< sections.length; i++) {
+            sections[i].clear();
+        }
+    };
+
     that.add_sections(spec.sections);
 
     that.details_facet_create_content = that.create_content;
diff --git a/install/ui/entity.js b/install/ui/entity.js
index 704b5c43b2ce7ef9dcc7221f5a73e4bf5df4bf15..82cbb11f61d7eaf0ef11687949156c19f7ea67fd 100644
--- a/install/ui/entity.js
+++ b/install/ui/entity.js
@@ -109,6 +109,9 @@ IPA.facet = function (spec) {
         that.header.load(data);
     };
 
+    that.clear = function() {
+    };
+
     that.needs_update = function() {
         return true;
     };
@@ -367,6 +370,10 @@ IPA.facet_header = function(spec) {
         }
     };
 
+    that.clear = function() {
+        that.load({});
+    };
+
     return that;
 };
 
@@ -589,9 +596,15 @@ IPA.entity = function (spec) {
             that.facet.create(facet_container);
         }
 
-        that.facet.show();
-        that.facet.header.select_tab();
-        that.facet.refresh();
+        if (that.facet.needs_update()) {
+            that.facet.clear();
+            that.facet.show();
+            that.facet.header.select_tab();
+            that.facet.refresh();
+        } else {
+            that.facet.show();
+            that.facet.header.select_tab();
+        }
     };
 
     that.get_primary_key_prefix = function() {
diff --git a/install/ui/host.js b/install/ui/host.js
index 992030527cda9b83cc06ef148dffd3e466ae67e7..b1d16e6457b544716db908cccb75b14b14727b37 100644
--- a/install/ui/host.js
+++ b/install/ui/host.js
@@ -567,6 +567,11 @@ IPA.host_keytab_widget = function(spec) {
         set_status(value ? 'present' : 'missing');
     };
 
+    that.clear = function() {
+        that.present_span.css('display', 'none');
+        that.missing_span.css('display', 'none');
+    };
+
     function set_status(status) {
         that.present_span.css('display', status == 'present' ? 'inline' : 'none');
         that.missing_span.css('display', status == 'missing' ? 'inline' : 'none');
@@ -720,6 +725,13 @@ IPA.host_password_widget = function(spec) {
         set_status(value ? 'present' : 'missing');
     };
 
+    that.clear = function() {
+        that.missing_span.css('display', 'none');
+        that.present_span.css('display', 'none');
+        var password_label = $('.button-label', that.set_password_button);
+        password_label.text('');
+    };
+
     function set_status(status) {
 
         that.status = status;
diff --git a/install/ui/search.js b/install/ui/search.js
index ab3a1cea44fbfbd0aa43fdab22da73003fd7e042..59a2ea891e3696f4c27c9162a8de55a2237e1c7c 100644
--- a/install/ui/search.js
+++ b/install/ui/search.js
@@ -165,8 +165,10 @@ IPA.search_facet = function(spec) {
     that.show = function() {
         that.facet_show();
 
+        var filter = IPA.nav.get_state(that.entity.name+'-filter');
+        that.old_filter = filter || '';
+
         if (that.filter) {
-            var filter = IPA.nav.get_state(that.entity.name+'-filter');
             that.filter.val(filter);
         }
     };
@@ -266,9 +268,8 @@ IPA.search_facet = function(spec) {
         var current_entity = entity;
         filter.unshift(IPA.nav.get_state(current_entity.name+'-filter'));
         current_entity = current_entity.get_containing_entity();
-        while(current_entity !== null){
-            filter.unshift(
-                IPA.nav.get_state(current_entity.name+'-pkey'));
+        while (current_entity !== null) {
+            filter.unshift(IPA.nav.get_state(current_entity.name+'-pkey'));
             current_entity = current_entity.get_containing_entity();
         }
 
@@ -286,6 +287,15 @@ IPA.search_facet = function(spec) {
         command.execute();
     };
 
+    that.clear = function() {
+        that.table.clear();
+    };
+
+    that.needs_update = function() {
+        var filter = IPA.nav.get_state(that.entity.name+'-filter') || '';
+        return that.old_filter !== '' || that.old_filter !== filter;
+    };
+
     // methods that should be invoked by subclasses
     that.search_facet_create_content = that.create_content;
 
diff --git a/install/ui/service.js b/install/ui/service.js
index 0dcfea3571f9c225b6149ca3b678b6a0157a8f3c..84764194098c15eb458d0b49c8c6309f91e6e74c 100644
--- a/install/ui/service.js
+++ b/install/ui/service.js
@@ -301,6 +301,11 @@ IPA.service_provisioning_status_widget = function (spec) {
         set_status(krblastpwdchange ? 'valid' : 'missing');
     };
 
+    that.clear = function() {
+        that.status_valid.css('display', 'none');
+        that.status_missing.css('display', 'none');
+    };
+
     function set_status(status) {
         that.status_valid.css('display', status == 'valid' ? 'inline' : 'none');
         that.status_missing.css('display', status == 'missing' ? 'inline' : 'none');
diff --git a/install/ui/user.js b/install/ui/user.js
index 69924429d98b4f6dcf74e32f0c5d72cc67a55062..eade0bbd2878bbf6e85a1822b6e54c3ce94a28fd 100644
--- a/install/ui/user.js
+++ b/install/ui/user.js
@@ -292,6 +292,11 @@ IPA.user_status_widget = function(spec) {
         }
     };
 
+    that.clear = function() {
+        that.link_span.css('display', 'none');
+        that.status_span.text('');
+    };
+
     that.show_activation_dialog = function() {
 
         var action = that.status_link.attr('href');
diff --git a/install/ui/widget.js b/install/ui/widget.js
index 7f2260c73156c1bf263b4d987edd404bc96ec92d..4d3a3524cb441e2c40f5dab1ed3c110e0ebda165 100644
--- a/install/ui/widget.js
+++ b/install/ui/widget.js
@@ -379,6 +379,9 @@ IPA.widget = function(spec) {
         error_link.css('display', 'none');
     };
 
+    that.clear = function() {
+    };
+
     that.set_enabled = function() {
     };
 
@@ -492,6 +495,11 @@ IPA.text_widget = function(spec) {
         }
     };
 
+    that.clear = function() {
+        that.input.val('');
+        that.display_control.text('');
+    };
+
     // methods that should be invoked by subclasses
     that.text_load = that.load;
 
@@ -756,6 +764,10 @@ IPA.multivalued_text_widget = function(spec) {
         that.set_dirty(false, index);
     };
 
+    that.clear = function() {
+        that.remove_rows();
+    };
+
     that.update = function(index) {
 
         var value;
@@ -858,6 +870,10 @@ IPA.checkbox_widget = function (spec) {
         return false;
     };
 
+    that.clear = function() {
+        that.input.attr('checked', false);
+    };
+
     that.checkbox_save = that.save;
     that.checkbox_load = that.load;
 
@@ -945,6 +961,10 @@ IPA.checkboxes_widget = function (spec) {
         }
     };
 
+    that.clear = function() {
+        $('input[name="'+that.name+'"]').attr('checked', false);
+    };
+
     // methods that should be invoked by subclasses
     that.checkboxes_update = that.update;
 
@@ -965,19 +985,18 @@ IPA.radio_widget = function(spec) {
 
         container.addClass('radio-widget');
 
+        var name = IPA.html_util.get_id(that);
+        that.selector = 'input[name="'+name+'"]';
+
         for (var i=0; i<that.options.length; i++) {
             var option = that.options[i];
 
-            // TODO: Use ID generator or accept ID from spec to avoid conflicts.
-            // Currently this ID is unique enough, but it will not work if the
-            // radio button is used multiple times for the same attribute, for
-            // example both in adder dialog and details facet.
-            var id = that.entity.name+'-'+that.name+'-'+i+'-radio';
+            var id = name+'-'+i;
 
             $('<input/>', {
                 id: id,
                 type: 'radio',
-                name: that.name,
+                name: name,
                 value: option.value
             }).appendTo(container);
 
@@ -991,7 +1010,7 @@ IPA.radio_widget = function(spec) {
             that.create_undo(container);
         }
 
-        var input = $('input[name="'+that.name+'"]', that.container);
+        var input = $(that.selector, that.container);
         input.change(function() {
             that.set_dirty(that.test_dirty());
         });
@@ -1008,20 +1027,20 @@ IPA.radio_widget = function(spec) {
     };
 
     that.save = function() {
-        var input = $('input[name="'+that.name+'"]:checked', that.container);
+        var input = $(that.selector+':checked', that.container);
         if (!input.length) return [];
         return [input.val()];
     };
 
     that.update = function() {
 
-        $('input[name="'+that.name+'"]', that.container).each(function() {
+        $(that.selector, that.container).each(function() {
             var input = this;
             input.checked = false;
         });
 
         var value = that.values && that.values.length ? that.values[0] : '';
-        var input = $('input[name="'+that.name+'"][value="'+value+'"]', that.container);
+        var input = $(that.selector+'[value="'+value+'"]', that.container);
         if (input.length) {
             input.attr('checked', true);
         }
@@ -1032,6 +1051,10 @@ IPA.radio_widget = function(spec) {
         return false;
     };
 
+    that.clear = function() {
+        $(that.selector, that.container).attr('checked', false);
+    };
+
     // methods that should be invoked by subclasses
     that.radio_create = that.create;
     that.radio_save = that.save;
@@ -1104,6 +1127,10 @@ IPA.select_widget = function(spec) {
         $('option', that.select).remove();
     };
 
+    that.clear = function() {
+        that.empty();
+    };
+
     // methods that should be invoked by subclasses
     that.select_load = that.load;
     that.select_save = that.save;
@@ -1166,6 +1193,10 @@ IPA.textarea_widget = function (spec) {
         that.input.val(value);
     };
 
+    that.clear = function() {
+        that.input.val('');
+    };
+
     return that;
 };
 
@@ -1639,6 +1670,12 @@ IPA.table_widget = function (spec) {
         }
     };
 
+    that.clear = function() {
+        that.empty();
+        that.summary.text('');
+    };
+
+    //column initialization
     if (spec.columns) {
         for (var i=0; i<spec.columns; i++) {
             that.create_column(spec.columns[i]);
@@ -1893,6 +1930,11 @@ IPA.combobox_widget = function(spec) {
         that.list.empty();
     };
 
+    that.clear = function() {
+        that.input.val('');
+        that.remove_options();
+    };
+
     return that;
 };
 
@@ -2008,6 +2050,11 @@ IPA.entity_link_widget = function(spec) {
         }).execute();
     };
 
+    that.clear = function() {
+        that.nonlink.text('');
+        that.link.text('');
+    };
+
 
     return that;
 };
@@ -2081,3 +2128,16 @@ IPA.observer = function(spec) {
 
     return that;
 };
+
+IPA.html_util = function() {
+
+    var that = {};
+    that.id_count = 0;
+
+    that.get_id = function(widget) {
+        that.id_count++;
+        return widget.entity.name+'-'+widget.name+'-'+that.id_count;
+    };
+
+    return that;
+}();
-- 
1.7.6.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to