Lior Vernia has posted comments on this change. Change subject: webadmin: AddRemoveRowWidget to enable/disable rows ......................................................................
Patch Set 5: (2 comments) http://gerrit.ovirt.org/#/c/29228/5/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 71: Line 72: @Override Line 73: protected void toggleGhost(KeyValueLineModel value, KeyValueLineWidget widget, boolean becomingGhost) { Line 74: super.toggleGhost(value, widget, becomingGhost); Line 75: widget.setEnabled(!becomingGhost); > 1. Consider moving the code to the parent toggleGhost 1. The generic toggleGhost() has no knowledge of what the widget looks like and which of its UI fields should be disabled when becoming a ghost entry. So this logic cannot sit there. 2. toggleGhost() can only be called if the widget is enabled. It is only triggered following ValueChangeEvent on the widget, or programmatically from AddRemoveRowWidget - where in the up-to-date code it's only called if the widget is enabled. Line 76: widget.keyField.setEnabled(true); Line 77: } Line 78: Line 79: @Override Line 72: @Override Line 73: protected void toggleGhost(KeyValueLineModel value, KeyValueLineWidget widget, boolean becomingGhost) { Line 74: super.toggleGhost(value, widget, becomingGhost); Line 75: widget.setEnabled(!becomingGhost); Line 76: widget.keyField.setEnabled(true); > should consider also 'enabled' Same. Line 77: } Line 78: Line 79: @Override Line 80: 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: 5 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
