Vojtech Szocs has posted comments on this change. Change subject: userportal, webadmin: Number field reporting wrong error ......................................................................
Patch Set 8: (3 comments) https://gerrit.ovirt.org/#/c/37244/8/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AbstractValidatedWidgetWithLabel.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/AbstractValidatedWidgetWithLabel.java: Line 114: } Line 115: Line 116: public AbstractValidatedWidgetWithLabel(W contentWidget) { Line 117: this(contentWidget, new VisibilityRenderer.SimpleVisibilityRenderer()); Line 118: addStateUpdateHandler(); I missed this in my first review - addStateUpdateHandler() is called due to this() (the other constructor above) so calling addStateUpdateHandler() here is not needed, I guess? Line 119: } Line 120: Line 121: @Override Line 122: protected void initWidget(Widget wrapperWidget) { Line 298: Line 299: @Override Line 300: public void markAsValid() { Line 301: if (editorStateValid) { Line 302: super.markAsValid(); Just wondering, why put only super.markAsValid() inside "if (editorStateValid)" condition? Line 303: } Line 304: labelTooltip.setText(labelConfiguredTooltip); Line 305: labelTooltip.reconfigure(); Line 306: contentWidgetContainerTooltip.setText(contentWidgetContainerConfiguredTooltip); https://gerrit.ovirt.org/#/c/37244/8/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/generic/EntityModelTextBox.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/widget/editor/generic/EntityModelTextBox.java: Line 34: * Return the parsed value, or null if the field is empty or parsing fails. If the validity of the box changes Line 35: * fire an {@code EditorStateUpdateEvent}, so interested parties can handle it. Line 36: */ Line 37: @Override Line 38: public T getValue() { Hm, I'd suggest a small improvement here, up to you: T value = null; boolean originalValidState = isValid; isValid = false; try { value = getValueOrThrow(); isValid = true; } finally { if (originalValidState != isValid) { fireEvent(new EditorStateUpdateEvent(isValid)); } return value; } Line 39: T value = null; Line 40: boolean originalValidState = isValid; Line 41: try { Line 42: value = getValueOrThrow(); -- To view, visit https://gerrit.ovirt.org/37244 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3702cb370528a0baf6a176fa89cd736596a10ff2 Gerrit-PatchSet: 8 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Alexander Wels <[email protected]> Gerrit-Reviewer: Alexander Wels <[email protected]> Gerrit-Reviewer: Einav Cohen <[email protected]> Gerrit-Reviewer: Greg Sheremeta <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
