Alexander Wels has posted comments on this change. Change subject: webadmin: Fix action button regression fix ......................................................................
Patch Set 1: (1 comment) http://gerrit.ovirt.org/#/c/23920/1/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/action/AbstractActionPanel.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/action/AbstractActionPanel.java: Line 101: // List of buttons that only show in the tool-bar. Line 102: private final List<ActionButtonDefinition<T>> toolbarOnlyActionButtonList = Line 103: new ArrayList<ActionButtonDefinition<T>>(); Line 104: // List of original visibility state for each button Line 105: private final Map<Widget, Boolean> originallyVisible = new HashMap<Widget, Boolean>(); > Consider using a Set instead, and insert only buttons that were originally The problem is that when first loaded, there are buttons that are visible, which are then later set to invisible by the addInitializeHandler. So instead of being able to do this: originallyVisible.put(widget, true/false). I would have to do this: if (widget.visible) { originallyVisible.remove(widget); } else { originallyVisible.put(widget); } Which is a lot more code for little to no gain. Line 106: Line 107: private final SearchableModelProvider<T, ?> dataProvider; Line 108: private final EventBus eventBus; Line 109: -- To view, visit http://gerrit.ovirt.org/23920 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib3c5ccb0a8732b206c94591b930fc6a01456a47d Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Lior Vernia <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[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
