[Engine-devel] Sorting in tabs

[Eli Mesika]

----- Original Message -----
> From: "Einav Cohen" <ecohen at redhat.com>
> To: "Lior Vernia" <lvernia at redhat.com>
> Cc: "Eli Mesika" <emesika at redhat.com>, engine-devel at ovirt.org, "Vojtech Szocs" <vszocs at redhat.com>, "Alexander Wels"
> <awels at redhat.com>, "Daniel Erez" <derez at redhat.com>, "Gilad Chaplik" <gchaplik at redhat.com>, "Alona Kaplan"
> <alkaplan at redhat.com>, "Tomas Jelinek" <tjelinek at redhat.com>
> Sent: Thursday, June 27, 2013 4:42:18 PM
> Subject: Re: [Engine-devel] Sorting in tabs
> 
> > ----- 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.
> 
> 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.
> 
> 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 think that when we will implement column sorting it will fulfill all UI sorting requirements 
> 
> > 
> > > 
> > > I believe that the correct thing to do is to "attach" the GUI sorting
> > > mechanism
> > > to the one in the search mechanism.
> > > 
> > > thoughts?

But this should be visible , i.e. the search syntax should be changed to reflect this kind of sorting.

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