On 11/01/2011 11:31 PM, Endi Sukma Dewata wrote:
On 11/1/2011 7:37 AM, Petr Vobornik wrote:
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
On the second thought, this might not be sufficient to detect the
changes in the list page. Try changing an attribute in an entry, then go
back to the list page, the list page will not show the updated attribute
because the filter is not changed.
I think we should remove the needs_update() from the search facet so it
will always refresh, but we probably can keep it for association facet.
What do you think? Sorry about that.
Changed to refresh always. Clearing always or not clearing at all seems
wrong. When the pkey doesn't change, only small portion of values
can/will change, so it's probably not so wrong to show the previous
list, but when the filter is changed the results will be probably much
different and the clear is needed. What do you think? -> Added
needs_clear method which clears if the filter changes (basically renamed
needs_update).
There are probably better solutions, but let's do this separately.
In future we can build a mechanism for subscribing to events from
different facets and doing appropriate actions.
Something like:
var refresh_search_on_save = function(spec) {
var that = {};
that.register = function() {
that.entity = IPA.get_entity(spec.entity);
that.details_facet = that.entity.get_facet(spec.facet ||
'details');
that.search_facet = that.entity.get_facet(spec.search_facet ||
'search');
that.details_facet.on_save.attach(that.on_save, that);
};
that.on_save = function() {
that.search_facet.set_needs_refresh(true);
};
return that;
};
So the facets won't be dependent on each other.
5) Added ID generator, using in radio_widget, same reason as #4.
The get_id() method (might be better called get_next_id() or
generate_id()) doesn't really need to take a widget parameter. The
id_count should be unique enough. If you want, it can take an optional
prefix so the ID will be like '<prefix>-<id>'. It will make it more
usable for other things not just widgets.
changed to get_next_id(prefix).
9. The facet header's clear() calls load() with empty data. The load()
will display the facet group label using facet's pkey. Since this is
called before refresh(), sometimes you'll see 'undefined' or the old
pkey. I think the code in entity.js:351-354 should check if the data is
empty it should clear the label.
Fixed
10. Instead of emptying button label in host.js:731-732, it's probably
better to reset it to its initial value:
var password_label = $('.button-label', that.set_password_button);
password_label.text(IPA.messages.objects.host.password_set_button);
Seems more proper to clean the label. If the label is set to
...host.password_set_button it will always shows before load "set OTP"
after load it can change to "reset OTP" which is wrong behavior.
--
Petr Vobornik
From b239957ae77de87bab163f3f43ca337d7f7bee33 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 | 28 +++++++++++++---
install/ui/host.js | 12 +++++++
install/ui/search.js | 20 +++++++++--
install/ui/service.js | 5 +++
install/ui/user.js | 5 +++
install/ui/widget.js | 80 +++++++++++++++++++++++++++++++++++++++------
9 files changed, 173 insertions(+), 28 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..ace44c3c1fef43e8a8700038c5305391ae3106a4 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;
};
@@ -336,6 +339,11 @@ IPA.facet_header = function(spec) {
that.load = function(data) {
if (!that.facet.disable_facet_tabs) {
+ var pkey = that.facet.pkey;
+ if(!pkey || !data) {
+ pkey = '';
+ }
+
var facet_groups = that.facet.entity.facet_groups.values;
for (var i=0; i<facet_groups.length; i++) {
var facet_group = facet_groups[i];
@@ -345,7 +353,7 @@ IPA.facet_header = function(spec) {
var label = facet_group.label;
if (label) {
- label = label.replace('${primary_key}', that.facet.pkey);
+ label = label.replace('${primary_key}', pkey);
var label_container = $('.facet-group-label', span);
label_container.text(label);
@@ -356,7 +364,7 @@ IPA.facet_header = function(spec) {
var facet = facets[j];
var link = $('li[name='+facet.name+'] a', span);
- var values = data[facet.name];
+ var values = data ? data[facet.name] : null;
if (values) {
link.text(facet.label+' ('+values.length+')');
} else {
@@ -367,6 +375,10 @@ IPA.facet_header = function(spec) {
}
};
+ that.clear = function() {
+ that.load();
+ };
+
return that;
};
@@ -589,9 +601,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..a8f897ab597c2dd38208351e5768029133ad7085 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,17 @@ IPA.search_facet = function(spec) {
command.execute();
};
+ that.clear = function() {
+ if(that.needs_clear()) {
+ that.table.clear();
+ }
+ };
+
+ that.needs_clear = 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..fb87c89935d3c72d444a08e18580bb099134bc3f 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_next_id(that.entity.name+'-'+that.name+'-');
+ 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_next_id = function(prefix) {
+ that.id_count++;
+ return prefix ? prefix + that.id_count : 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
