[Freeipa-devel] [PATCH] 029 Page is cleared before it is visible

[Endi Sukma Dewata]

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