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

[Petr Vobornik]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pvoborni-0029-2-Page-is-cleared-before-it-is-visible.patch
Type: text/x-patch
Size: 17747 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20111102/a46dcfae/attachment.bin>