Lior Vernia has posted comments on this change. Change subject: webadmin: Render client-side sorting more robust ......................................................................
Patch Set 3: Verified-1 (1 comment) There's still an issue with refresh due to the clearing of the insertion index Map. Will push a new patchset once I think about it more thoroughly. http://gerrit.ovirt.org/#/c/28268/3/frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java File frontend/webadmin/modules/uicommonweb/src/main/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModel.java: Line 90: } Line 91: Line 92: insertionIndex.clear(); Line 93: SortedSet<T> sortedItems = new TreeSet<T>(comparator) { Line 94: private int counter = 0; > What about allAll, clear(), removeAll()? What about the iterators? There ar I think all implementations of addAll() call add(), and same with removeAll(), though I can verify. And what about iterators? Should work properly to my understanding. And yes, we're gaining two things. The first is the stableness of the sort (across refreshes, see my comment on this entire patchset; a List would not achieve that by default), which I claim leads to a better UX. The second is robustness in the face of developers manipulating the collection of items not via setItems(), which is perfectly legitimate manipulation. What do you mean by "at this point"? We're still only talking about a dozen lines of code. Line 95: Line 96: @Override Line 97: public boolean add(T e) { Line 98: insertionIndex.put(e, counter++); // item will be placed following all existing "equal" items -- To view, visit http://gerrit.ovirt.org/28268 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iffd1ce7763e516ad109fdae5d584b183ee44117c Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
