Lior Vernia has posted comments on this change. Change subject: webadmin: adding label tag to Host->Interface subtab ......................................................................
Patch Set 3: (3 comments) Other than the inline comments, why not put the label next to the network name rather than the interface/bond name? It'll look more uniform, take less code change and be just as correct, won't it? http://gerrit.ovirt.org/#/c/27898/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostInterfaceView.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/section/main/view/tab/host/SubTabHostInterfaceView.java: Line 157: hostInterfaceForm = new HostInterfaceForm(getDetailModel()); Line 158: contentPanel.remove(contentPanel.getWidgetCount() - 1); Line 159: contentPanel.add(hostInterfaceForm); Line 160: Line 161: contentPanel.getParent().getElement().getStyle().setOverflowX(Overflow.AUTO); This should work if you put it in the constructor after initWidget(), shouldn't it? It shouldn't be invoked upon selection of a main tab item. Line 162: } Line 163: Line 164: @Override Line 165: public void setRefreshButtonVisibility(boolean visible) { http://gerrit.ovirt.org/#/c/27898/3/frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/label/LabelWithToolTip.java File frontend/webadmin/modules/webadmin/src/main/java/org/ovirt/engine/ui/webadmin/widget/label/LabelWithToolTip.java: Line 23: Line 24: public LabelWithToolTip(String text, int length) { Line 25: this(text); Line 26: Line 27: if (length > -1 && text.length() > length) { I think you can get rid of the first condition now, due to the change above. Line 28: setText(text.substring(0, length - 3) + "..."); //$NON-NLS-1$ Line 29: } Line 30: } Line 31: Line 32: public LabelWithToolTip(String text) { Line 33: this(SafeHtmlUtils.fromTrustedString(text)); Line 34: } Line 35: Line 36: public LabelWithToolTip(SafeHtml text) { I think a lot of the changes in this file can be avoided. Why not keep the String constructor the way it was, and only call here this(text.asString())? You won't have to change the type of the title member, nor add the setTitle() overload, etc. Line 37: super(text); Line 38: Line 39: tooltipPanel.setWidget(tooltip); Line 40: tooltipPanel.getElement().getStyle().setZIndex(1); -- To view, visit http://gerrit.ovirt.org/27898 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I997d85d894d79f14402157fcc51a0cdbfe2ff056 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alona Kaplan <alkap...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@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
