Yevgeny Zaspitsky has posted comments on this change. Change subject: webadmin: Add warning to Setup networks dialog ......................................................................
Patch Set 6: (4 comments) http://gerrit.ovirt.org/#/c/27463/6/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostSetupNetworksPopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostSetupNetworksPopupView.java: Line 154: // this is called after both networks and nics were retrieved Line 155: HostSetupNetworksModel model = (HostSetupNetworksModel) sender; Line 156: List<LogicalNetworkModel> networks = model.getNetworks(); Line 157: List<NetworkInterfaceModel> nics = model.getNics(); Line 158: lastStatusLine.setEmptyStatusText(); > I'm suggesting to drop this, see my comment below. Done Line 159: updateNetworks(networks); Line 160: updateNics(nics); Line 161: // mark as rendered Line 162: rendered = true; Line 175: final String message; Line 176: Line 177: if (candidate == null) { Line 178: message = constants.noValidActionSetupNetwork(); Line 179: statusLine = errorStatusLine; > I would instantiate the status lines here instead of above. those objects are immutable and can be created in the constructor, so I'd prefer to leave it as is. (was discussed and agreed wit you) Line 180: } else { Line 181: message = candidate.getMessage(op1, op2); Line 182: Line 183: if (candidate.isErroneousOperation()) { Line 193: Line 194: if (!evtArgs.isDrop()) { Line 195: statusLine.setStatusText(message); Line 196: } Line 197: statusLine.setStatusStyle(); > P.S. please note that status.setFadeText() has some delay and animation ass Done Line 198: lastStatusLine = statusLine; Line 199: } Line 200: }); Line 201: Line 242: Line 243: void setEmptyStatusText(); Line 244: } Line 245: Line 246: private abstract class AbstractStatusLine implements StatusLine { > I don't see the need for a class hierarchy here. You could have one class o Done Line 247: private final String panelStyleName; Line 248: private final String labelStyleName; Line 249: Line 250: AbstractStatusLine(String panelStyleName, String labelStyleName) { -- To view, visit http://gerrit.ovirt.org/27463 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I35f0e019b6f77ab90fec89f614b49db589602353 Gerrit-PatchSet: 6 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
