New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation not always fired #165

Closed
czyzby opened this Issue May 1, 2016 · 7 comments

Comments

Projects
None yet
2 participants
@czyzby
Contributor

czyzby commented May 1, 2016

There seems to be a bug with VisValidatableTextField and forms when "changing" its value to, well, the same value - form is not refreshed, so even if the data is currently valid, error message will still be displayed.

Reproduction:

  • Add text field with empty directory filter.
  • Choose a non-empty directory with file chooser, set directory path as field content.
  • Delete the content of non-empty directory.
  • Choose the same directory again, set as field content.
  • Form still displays the error, yet field no longer has the error border.

See example below. Text field now contains path to a valid directory and no longer has error border, yet error message is still displayed. Changing any field included in form fixes this issue, but the refresh should be automatic, not manual.
Invalid form

@kotcrab kotcrab added bug ui labels May 1, 2016

@kotcrab

This comment has been minimized.

Show comment
Hide comment
@kotcrab

kotcrab May 1, 2016

Owner

Can confirm, this is a bit troubling, VisTextField (the same would go for TextField) does not consider changing the text to the same one as it was before as actual change so it doesn't send change event. Which doesn't trigger change listener added by form validator and form status isn't updated.

(Vis)TextField#setText:

public void setText (String str) {
    if (str == null) str = "";
    if (str.equals(text)) return;
    //...
}

VisValidatableTextField performs validation regardless of that so its status is updated correctly.

Owner

kotcrab commented May 1, 2016

Can confirm, this is a bit troubling, VisTextField (the same would go for TextField) does not consider changing the text to the same one as it was before as actual change so it doesn't send change event. Which doesn't trigger change listener added by form validator and form status isn't updated.

(Vis)TextField#setText:

public void setText (String str) {
    if (str == null) str = "";
    if (str.equals(text)) return;
    //...
}

VisValidatableTextField performs validation regardless of that so its status is updated correctly.

@czyzby

This comment has been minimized.

Show comment
Hide comment
@czyzby

czyzby May 1, 2016

Contributor

Well, you could either always fire change events in your overridden setText method (after all - user requested to change the content) or add a boolean setting that allows you to decide whether change events should be fired in such situation. Something like alwaysFireChangeEvent, although you could go for something less verbose.

Contributor

czyzby commented May 1, 2016

Well, you could either always fire change events in your overridden setText method (after all - user requested to change the content) or add a boolean setting that allows you to decide whether change events should be fired in such situation. Something like alwaysFireChangeEvent, although you could go for something less verbose.

@kotcrab kotcrab closed this in 0a2123c May 1, 2016

@kotcrab

This comment has been minimized.

Show comment
Hide comment
@kotcrab

kotcrab May 1, 2016

Owner

Alright done, I added VisTextField property for controlling this. It should work now, no change is needed on your side.

Owner

kotcrab commented May 1, 2016

Alright done, I added VisTextField property for controlling this. It should work now, no change is needed on your side.

@czyzby

This comment has been minimized.

Show comment
Hide comment
@czyzby

czyzby May 1, 2016

Contributor

Except it's true by default (which is reasonable, as it matches old behavior), so I do have to change text field settings. ;) Still, thanks for the quick fix.

Contributor

czyzby commented May 1, 2016

Except it's true by default (which is reasonable, as it matches old behavior), so I do have to change text field settings. ;) Still, thanks for the quick fix.

@czyzby

This comment has been minimized.

Show comment
Hide comment
@czyzby

czyzby May 1, 2016

Contributor

I can confirm that forms work as expected when this property is set to false.

Contributor

czyzby commented May 1, 2016

I can confirm that forms work as expected when this property is set to false.

@kotcrab

This comment has been minimized.

Show comment
Hide comment
@kotcrab

kotcrab May 1, 2016

Owner

Yes it's true for VisTextField to match old behavior but VisValidatableTextField sets it to false as default. See VisValidatableTextField#init.

Owner

kotcrab commented May 1, 2016

Yes it's true for VisTextField to match old behavior but VisValidatableTextField sets it to false as default. See VisValidatableTextField#init.

@czyzby

This comment has been minimized.

Show comment
Hide comment
@czyzby

czyzby May 1, 2016

Contributor

Oh, I see, I haven't checked out the whole commit. Now that I think about it, that seems to be the correct approach. Thanks.

Contributor

czyzby commented May 1, 2016

Oh, I see, I haven't checked out the whole commit. Now that I think about it, that seems to be the correct approach. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment