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.


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


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.

There are probably better solutions, but let's do this separately.

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.

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.

You can reload the page too, but I agree we probably need a refresh button, and possibly for other facets too. I'll open a ticket.

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.

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);

Endi S. Dewata

Freeipa-devel mailing list

Reply via email to