Tomas Jelinek has posted comments on this change. Change subject: userportal\webadmin: cluster parameters override ......................................................................
Patch Set 21: (5 comments) http://gerrit.ovirt.org/#/c/31582/21/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/ListModelTypeAheadChangeableListBox.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/ListModelTypeAheadChangeableListBox.java: Line 37: boolean isEmptyReplace = Line 38: StringUtils.isEmpty(value) || value.equals(getNullReplacementText()); Line 39: // handle null replacement Line 40: asSuggestBox().setValue(isEmptyReplace ? getNullReplacementText() : value, Line 41: fireEvents); //$NON-NLS-1$ no need for this non-nls here Line 42: asSuggestBox().getElement().getStyle().setColor(isEmptyReplace ? "gray" : "black"); //$NON-NLS-1$ $NON-NLS-2$ Line 43: } Line 44: Line 45: @Override Line 49: suggestBox.setText(lastText); Line 50: } Line 51: Line 52: public String getNullReplacementText() { Line 53: return nullReplacementText; do you need this getter to be public? I'd hide it as private or delete it completely... Line 54: } Line 55: Line 56: public void setNullReplacementText(String nullReplacementText) { Line 57: this.nullReplacementText = nullReplacementText; Line 52: public String getNullReplacementText() { Line 53: return nullReplacementText; Line 54: } Line 55: Line 56: public void setNullReplacementText(String nullReplacementText) { also the setter seems not that useful, better to have it in the constructor and delete this. Line 57: this.nullReplacementText = nullReplacementText; Line 58: } http://gerrit.ovirt.org/#/c/31582/21/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmRunOncePopupWidget.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/uicommon/popup/vm/VmRunOncePopupWidget.java: Line 408: @Override Line 409: public String getDisplayStringNullSafe(String data) { Line 410: return typeAheadNameTemplateNullSafe(data); Line 411: } Line 412: }, don't you need to set the replacement string also here? Line 413: false); Line 414: customCpu = new ListModelTypeAheadChangeableListBoxEditor( Line 415: new ListModelTypeAheadChangeableListBoxEditor.NullSafeSuggestBoxRenderer() { Line 416: Line 417: @Override Line 418: public String getDisplayStringNullSafe(String data) { Line 419: return typeAheadNameTemplateNullSafe(data); Line 420: } Line 421: }, same Line 422: false); Line 423: } Line 424: Line 425: void initBootSequenceBox() { -- To view, visit http://gerrit.ovirt.org/31582 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia288e46f38c03935ec9b0817833b1921f15ec9f8 Gerrit-PatchSet: 21 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eldan Shachar <eshac...@redhat.com> Gerrit-Reviewer: Eldan Shachar <eshac...@redhat.com> Gerrit-Reviewer: Tomas Jelinek <tjeli...@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
