Vojtech Szocs has posted comments on this change.

Change subject: userportal, webadmin: Number field reporting wrong error
......................................................................


Patch Set 7:

(3 comments)

https://gerrit.ovirt.org/#/c/37244/7/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 65:                 public void onValueChange(ValueChangeEvent<T> event) {
Line 66:                     // Set value in model
Line 67:                     if (ctx.canSetInModel()) {
Line 68:                         boolean editorValid = true;
Line 69:                         if (event.getSource() instanceof 
EntityModelTextBox) {
Can we use some interface (that would be implemented by EntityModelTextBox) in 
order not to lock ourselves into EntityModelTextBox?
Line 70:                             editorValid = 
((EntityModelTextBox<?>)event.getSource()).isStateValid();
Line 71:                         }
Line 72:                         if (editorValid) {
Line 73:                             ctx.setInModel(event.getValue());


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;
Line 97:                             if (editor instanceof EntityModelTextBox) {
Same comment as above.

(Maybe also consider extracting common "check if valid and set in model" logic 
into some method.)
Line 98:                                 editorValid = 
((EntityModelTextBox<?>)editor).isStateValid();
Line 99:                             }
Line 100:                             if (editorValid) {
Line 101:                                 ctx.setInModel(editor.getValue());


Line 226:     void onIsValidPropertyChange(HasValidation editor, EntityModel 
model) {
Line 227:         if (model.getIsValid()) {
Line 228:             editor.markAsValid();
Line 229:         } else {
Line 230:             //The entity validator will mark the entity valid before 
running the validation
Maybe a silly question, but would it help if we modified that entity validator 
behavior instead of fixing it here?
Line 231:             //this will cause the editor to be marked valid if it was 
invalid due to an entity validation
Line 232:             //error. Then if the validation is invalid again this 
will update the error message. So there is
Line 233:             //no possibility to go from one invalid reason to another 
without the editor message being updated.
Line 234:             if (editor.isValid()) {


-- 
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: 7
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Alexander Wels <aw...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Greg Sheremeta <gsher...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@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

Reply via email to