Tomas Jelinek has posted comments on this change. Change subject: frontend: Adjust console tooltips ......................................................................
Patch Set 2: (1 comment) http://gerrit.ovirt.org/#/c/24249/2/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/ConsolePopupView.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/view/popup/ConsolePopupView.java: Line 366: rdpInvocationLabel.addStyleName(style.labelStyle()); Line 367: } Line 368: Line 369: private String createSpiceInvocationInfo() { Line 370: return createKeyValHtmlRows( I would prefer a bit more OO way, something like: new KeyValue(key, value). append(new KeyValue(key2, val2)) ... .toString Than the KeyValue would be an inner class which looks like this: class KeyValue { private String html; public KeyValue(String key, String value) { html = "<b>" + key + "</b>" + value; } public KeyValue append(KeyValue other) { html += "<br />" + other.toString(); return this; } public String toString() { return html; } } BTW there is no real need to worry about StringBuilder vs "+" in the GWT since it anyway compiles to JS and it is hard to tell which way is faster (in some versions of some browser the StringBuilder, in some the "+" but the difference in this small amount of concats is not significant...) Line 371: constants.auto(), constants.spiceInvokeAuto(), Line 372: constants.nativeClient(), constants.consoleInvokeNative(), Line 373: constants.browserPlugin(), constants.spiceInvokePlugin(), Line 374: constants.spiceHtml5(), constants.spiceInvokeHtml5() -- To view, visit http://gerrit.ovirt.org/24249 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2459521e7ba303918a0992ffd95398b0c42f03df Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <[email protected]> Gerrit-Reviewer: Frank Kobzik <[email protected]> Gerrit-Reviewer: Tomas Jelinek <[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
