Greg Sheremeta has posted comments on this change. Change subject: userportal, webadmin: Number field reporting wrong error ......................................................................
Patch Set 6: (6 comments) great patch. just some nitpicks. http://gerrit.ovirt.org/#/c/37244/6/frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/editor/UiCommonEditorVisitor.java File frontend/webadmin/modules/gwt-common/src/main/java/org/ovirt/engine/ui/common/editor/UiCommonEditorVisitor.java: Line 64: @Override Line 65: public void onValueChange(ValueChangeEvent<T> event) { Line 66: // Set value in model Line 67: if (ctx.canSetInModel()) { Line 68: boolean editorValid = true; this looks like a big change. Are we going from always setting values into the model ---> only setting if valid? Line 69: if (event.getSource() instanceof EntityModelTextBox) { Line 70: editorValid = ((EntityModelTextBox<?>)event.getSource()).isStateValid(); Line 71: } Line 72: if (editorValid) { Line 92: public void onKeyPress(KeyPressEvent event) { Line 93: if (KeyCodes.KEY_ENTER == event.getNativeEvent().getKeyCode()) { Line 94: // Set value in model Line 95: if (ctx.canSetInModel()) { Line 96: boolean editorValid = true; same as above Line 97: if (editor instanceof EntityModelTextBox) { Line 98: editorValid = ((EntityModelTextBox<?>)editor).isStateValid(); Line 99: } Line 100: if (editorValid) { Line 228: editor.markAsValid(); Line 229: } else { Line 230: //The validator will set the entities to be valid before running checks Line 231: //So there is no possibility to go from one error to another without this Line 232: //updating the error message. sorry, don't quite understand your comment here Line 233: if (editor.isValid()) { Line 234: editor.markAsInvalid(model.getInvalidityReasons()); Line 235: } Line 236: } http://gerrit.ovirt.org/#/c/37244/6/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 56: String contentWidget(); Line 57: } Line 58: Line 59: //We need to store the valid state of the editor so that when the model validator Line 60: //runs and the editor is not valid (due to a parsing error). The editor doesn't get "error). The " --> error), the Line 61: //reset by the model. Line 62: private boolean editorStateValid = true; Line 63: Line 64: private final W contentWidget; http://gerrit.ovirt.org/#/c/37244/6/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 29: } Line 30: Line 31: /** Line 32: * Return the parsed value, or null if the field is empty or parsing fails. If the parsing fails Line 33: * fire a parsing failed event, so interested parties can handle it. suggest tweaking this comment a bit. Looks like the EditorStateUpdateEvent is fired whenever the validity changes -- not just if there is a failure. Line 34: */ Line 35: @Override Line 36: public T getValue() { Line 37: T value = null; Line 32: * Return the parsed value, or null if the field is empty or parsing fails. If the parsing fails Line 33: * fire a parsing failed event, so interested parties can handle it. Line 34: */ Line 35: @Override Line 36: public T getValue() { +1 nice clear method :) Line 37: T value = null; Line 38: boolean originalValidState = isValid; Line 39: try { Line 40: value = getValueOrThrow(); -- To view, visit http://gerrit.ovirt.org/37244 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3702cb370528a0baf6a176fa89cd736596a10ff2 Gerrit-PatchSet: 6 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: Tal Nisan <[email protected]> Gerrit-Reviewer: Vojtech Szocs <[email protected]> Gerrit-Reviewer: [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
