[Engine-devel] Sorting in tabs

Thu Jun 27 14:38:04 UTC 2013

On 27/06/13 16:42, Einav Cohen wrote:
>> ----- Original Message -----
>> From: "Lior Vernia" <lvernia at redhat.com>
>> Sent: Thursday, June 27, 2013 8:53:59 AM
>>
>>
>>
>> On 27/06/13 15:37, Einav Cohen wrote:
>>>> ----- Original Message -----
>>>> From: "Eli Mesika" <emesika at redhat.com>
>>>> Sent: Thursday, June 27, 2013 6:46:58 AM
>>>>
>>>>
>>>> ----- Original Message -----
>>>>> From: "Lior Vernia" <lvernia at redhat.com>
>>>>> To: engine-devel at ovirt.org
>>>>> Sent: Thursday, June 27, 2013 10:12:33 AM
>>>>> Subject: [Engine-devel] Sorting in tabs
>>>>>
>>>>> Hello everyone (UI peeps in particular),
>>>>>
>>>>> I've pushed (not yet merged) a patch that would enable us to keep items
>>>>> in tabs (main/sub) sorted at all times by setting a comparator in
>>>>> SearchableListModel:
>>>>
>>>> But tabs includes only 100 records and supports paging , how you deal with
>>>> that ???
>>>
>>> if this is in the GUI level, then I assume that the comparator is simply
>>> comparing the
>>> items within the current page, and not "globally".
>>> so the sorting doesn't affect the set of items that is displayed in the
>>> page (it would
>>> be the same as before the sorting) - just their order.
>>
>> Yes, if I understand correctly how the paging works, Einav is correct -
>> only the items passed to the UI are sorted.
>>
>>> also: @Lior - what happens when the search query contains a "sort by" part?
>>> there is a chance that the behaivor would be unexpected in this case;
>>
>> Yes, I thought about this case, and it may result in a confusing user
>> experience if developers aren't careful. Together with the issue of
>> paging, this probably makes this sorting mechanism a better candidate
>> for use within subtabs rather than main tabs.
> 
> note that at some point, I think that we would want to introduce paging also to search-
> based sub-tabs - it will be useful especially for sub-tabs that potentially display a 
> large number of results (e.g. Disks sub-tab in Storage main tab). 
> In addition, at some point, we would want to get rid of the paging UI as it is now 
> (i.e. "next"/"prev" buttons at the top panel) and move to paging triggered by scroll 
> (i.e. have a very long grid, dynamically loaded as you continue to scroll - similar 
> to the behavior of some e-mail web-clients, for example). In this case, sorting on 
> the client side will make no sense at all (i.e. from the user perspective, only a 
> portion of a very large grid will be sorted, the other portions won't be).
> 
> So for now - yes, I think it makes sense to introduce your mechanism to all sub-tabs, 
> however in the long-term - we would probably want the search-based sub-tabs (which 
> will support paging) to move to search-based sorting, rather than GUI-based-sorting.

Sounds good to me. Let me just re-iterate that it is not mandatory to
set a comparator, so in technical terms it's not even necessary to
introduce it at once to all sub-tabs, if they're already sorting their
items some other way. It could happen gradually, and only if developers
find it more convenient. In either case, dropping the GUI sorting once
search-based sorting is implemented shouldn't be difficult.

> BTW (maybe the other GUI maintainers can help me with that one) - what about sub-tabs 
> that are not search-based (i.e. display results from a "regular" query or even from a 
> field within the selected item in the main grid, e.g. Applications in VM) - are these 
> managed via SearchableListModel as well? since the comparator mechanism *is* relevant 
> for them.

As far as I've seen, some are managed via SearchableListModel and some
aren't. Those that aren't are those that display non-trivial behaviour
upon receipt of the items to display (setItems() method) - often this
non-trivial behaviour is exactly sorting :) And if it's doing its job,
then there's no necessity to change it either. But anyway, I don't know
all of them, so I'd also love to hear GUI maintainers.

> Also: Worth mentioning "Bug 893999 - webadmin: please allow column sorting", which 
> requests to enable sorting when clicking on a grid-column header; when implementing 
> column-sorting, probably worth attaching your mechanism to it somehow (i.e. clicking on 
> a column header should set the relevant comparator in the relevant SearchableListModel).

I didn't want to say it, because if we upgrade to a newer version of GWT
then we could probably use their table column sorting. But this
mechanism could allow us to achieve this without upgrading, and it was
definitely sitting in the back of my head when I implemented it. All
that's needed is, as you said, to listen to table header clicks in the
view, and then appropriately set the comparator in the model.

>>
>>>
>>> I believe that the correct thing to do is to "attach" the GUI sorting
>>> mechanism
>>> to the one in the search mechanism.
>>>
>>> thoughts?
>>
>> This can be done, however I'm not sure there's much utility in it. Main
>> tabs are always sorted according to some default ordering even if one
>> was not entered in the search panel, and this sorting is also performed
>> consistently with respect to paging. So maybe the right thing to do
>> would be to just "block" the GUI sorting mechanism for main tabs (i.e.
>> override the setter method and make it no-op)?
> 
> yes, and related to what I mentioned above - at some point in the future, we'd might want 
> to block it for search-based sub-tabs as well.
> 
>>
>>>>
>>>>
>>>>>
>>>>> http://gerrit.ovirt.org/#/c/15846/
>>>>>
>>>>> If a comparator isn't set, then everything should behave as before. If a
>>>>> comparator is set, then from that moment on the tab items will be kept
>>>>> in a SortedSet, so that even if an item is added in a way that doesn't
>>>>> trigger an event (e.g. getItems().add()) the items will be kept sorted
>>>>> according to the given comparator. If the comparator is set to null,
>>>>> from that moment on the tab should revert to its old behaviour.
>>>>>
>>>>> You're most welcome to have a look and let me know if this might break
>>>>> something (remember though that it's not obligatory to set a comparator,
>>>>> so only possible breakage should be in generic flows).
>>>>>
>>>>> Feel free to use it once it's merged; along with SortedListModel, this
>>>>> should make sorting less painful. Just keep in mind that once you set a
>>>>> comparator, you can't cast getItems() to a List. This shouldn't be a
>>>>> problem in general, as mostly it's as useful (and probably more correct)
>>>>> to cast to a Collection.
>>>>>
>>>>> Lior.
>>>>> _______________________________________________
>>>>> Engine-devel mailing list
>>>>> Engine-devel at ovirt.org
>>>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>>
>>>> _______________________________________________
>>>> Engine-devel mailing list
>>>> Engine-devel at ovirt.org
>>>> http://lists.ovirt.org/mailman/listinfo/engine-devel
>>>>
>>