Lior Vernia has posted comments on this change. Change subject: webadmin: Fix server-side sort issue ......................................................................
Patch Set 5: (1 comment) Looks good in general, there's just one point that I feel uneasy about, see inline comment - it might be nonsense, I don't know the GUI infrastructure as well as you, please have a look though. http://gerrit.ovirt.org/#/c/28557/5/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/table/AbstractActionTable.java: Line 239: table.enableColumnWidthPersistence(clientStorage, dataProvider.getModel()); Line 240: } Line 241: Line 242: // When UiCommon models are (re)initialized, register search string change listener Line 243: eventBus.addHandler(UiCommonInitEvent.getType(), new UiCommonInitHandler() { Do these tables exist for the entire login session of the user? I'm not sure, I think they don't - but you know better me. If they indeed don't, then this handler should probably be removed sometime, so that events aren't relayed to destroyed objects. It most likely should be added as part of onLoad()/onAttach() and removed as part of onUnload/onDetach(). If they do, then this logic should probably be moved somewhere else - it sounds suboptimal to send these events to ALL tabs (rather than just the visible ones), especially considering the performance issues we already have with events. Probably somewhere in tabs' presenters (onReveal(), onHide()). Line 244: @Override Line 245: public void onUiCommonInit(UiCommonInitEvent event) { Line 246: addModelSearchStringChangeListener(dataProvider.getModel()); Line 247: } -- To view, visit http://gerrit.ovirt.org/28557 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7ce39a4e2b6323e6e0276a3a462a3f9da005344b Gerrit-PatchSet: 5 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Einav Cohen <[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
