On 11/2/2011 8:33 AM, Petr Vobornik wrote:
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).

OK. The search page feels more responsive this way. The term needs_clear() is a bit confusing because during facet.refresh() it also calls table.empty() which essentially the same as table.clear(). How about clear_before_show()? It's not important, we can do this later.

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.

Yes, that would be better, but I'd put the registration somewhere inside the entity class (to be created). This is also going to force early creation of the facets as opposed to lazy loading.

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

I think displaying no label at all would be better than showing incomplete label (without primary key), but we can do this later.

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.

I see your point. It might be better to just hide the button, but we can do this later.

Question about this one:

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

Did you notice any problem with the old radio name? The name was supposed to be used locally by the facet itself so it should not interfere with radios in other facets, whereas ID is global so it needs to be unique.

Regardless, ACK and pushed to master.

--
Endi S. Dewata

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

Reply via email to