Lior Vernia has posted comments on this change. Change subject: webadmin: AddRemoveRowWidget to enable/disable rows ......................................................................
Patch Set 3: (4 comments) http://gerrit.ovirt.org/#/c/29228/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AddRemoveRowWidget.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AddRemoveRowWidget.java: Line 169: : new AddRemoveRowPanel(widget, removeButton); Line 170: contentPanel.add(entry); Line 171: Line 172: final boolean ghost = isGhost(value); Line 173: setButtonsEnabled(widget, !ghost); > IIUC this call to 'setButtonsEnabled' is redundant, 'toggleEnabled' which i Put together with toggleGhost(), as they're called together. Also, toggleEnabled() was modified to only override if widget isn't ghost widget but needs to be disabled. Line 174: toggleGhost(value, widget, ghost); Line 175: toggleEnabled(widget, enabled); Line 176: widget.addValueChangeHandler(new ValueChangeHandler<T>() { Line 177: Line 368: */ Line 369: protected abstract void toggleGhost(T value, V widget, boolean becomingGhost); Line 370: Line 371: /** Line 372: * This method is called whenever a widget has to be enabled or disabled. It should implement the deftails of the > typo- details Not relevant anymore. Line 373: * widget's appearance as it becomes enabled or disabled. By default, this method enables and disables the buttons Line 374: * associated with an entry; it should be overridden in order to control how the widget itself looks. Line 375: * Line 376: * @param widget Line 378: * @param enabled Line 379: * true if the widget is being enabled, false if being disabled. Line 380: */ Line 381: protected void toggleEnabled(V widget, boolean enabled) { Line 382: getEntry(widget).setButtonsEnabled(enabled); > 1. Why don't you use setButtonsEnabled(widget, enabled)? Did semothing similar. Got rid of method by delegating responsibility to widget that implements HasEnabled. Line 383: } Line 384: http://gerrit.ovirt.org/#/c/29228/3/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/key_value/KeyValueWidget.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/form/key_value/KeyValueWidget.java: Line 77: Line 78: @Override Line 79: protected void toggleEnabled(KeyValueLineWidget widget, boolean enabled) { Line 80: super.toggleEnabled(widget, enabled); Line 81: widget.setEnabled(enabled); > If you"ll make the parents method more generic you won't need this overridd Done Line 82: } Line 83: Line 84: @Override Line 85: protected void onRemove(KeyValueLineModel value, KeyValueLineWidget widget) { -- To view, visit http://gerrit.ovirt.org/29228 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia9a6265b5c0eb27c3b99b98d88986762249acdbe Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Lior Vernia <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Lior Vernia <[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
