Lior Vernia has posted comments on this change. Change subject: webadmin: Add warning about changing display network in Edit Network screen ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/27301/2//COMMIT_MSG Commit Message: Line 3: AuthorDate: 2014-05-01 17:14:52 +0300 Line 4: Commit: Yevgeny Zaspitsky <yzasp...@redhat.com> Line 5: CommitDate: 2014-05-01 11:11:48 -0400 Line 6: Line 7: webadmin: Add warning about changing display network in Edit Network popup This looks too long for a patch title, I think the standard is ~50 characters. This looks even longer than 72, which is the max for a line in the description and would be passable for a title. Line 8: Line 9: Change-Id: I076156d70869dcd682ab742660d6271ee398f509 Line 10: Bug-Url: https://bugzilla.redhat.com/1078836 http://gerrit.ovirt.org/#/c/27301/2/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostInterfacePopupView.ui.xml File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/HostInterfacePopupView.ui.xml: Line 24: .customPropertiesStyle { Line 25: max-height: 100px; Line 26: } Line 27: Line 28: .displayNetworkChangeWarning { Apologies for misleading you, but this styling is too dramatic. It would be better to go along the lines of the noteHTML style in RemoveConfirmationPopupView.ui.xml (no bold, text not aligned to center, smaller font...). See if all of the contents of that style are required to make it look okay in this context, or just some of them. Line 29: color: red; Line 30: font-weight: bold; Line 31: text-align: center; Line 32: } http://gerrit.ovirt.org/#/c/27301/2/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/SetupNetworksInterfacePopupView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/popup/host/SetupNetworksInterfacePopupView.java: Line 50: customPropertiesPanel.setVisible(object.getCustomPropertiesModel().getIsAvailable()); Line 51: customPropertiesWidget.edit(object.getCustomPropertiesModel()); Line 52: Line 53: if (object.getNetwork().getSelectedItem().getCluster().isDisplay()) { Line 54: displayNetworkChangeWarning.setText(constants.changeDisplayNetworkWarning()); I would prefer to have this set in HostInterfacePopupView, with the rest of the localized strings, which would also obviate the need for the constants variable. Line 55: displayNetworkChangeWarning.setVisible(true); Line 56: } Line 57: } -- To view, visit http://gerrit.ovirt.org/27301 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I076156d70869dcd682ab742660d6271ee398f509 Gerrit-PatchSet: 2 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
