Skip to content
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

Update Binder to display updated validation status #9953

Closed
wants to merge 1 commit into from

Conversation

jcgueriaud
Copy link
Contributor

@jcgueriaud jcgueriaud commented Sep 8, 2017

Hello,

I have a strange behaviour. I use the binder with readbean and writeBeanIfValid.
doWriteIfValid doesn't show updated field status.
For example, an empty required field is not filled by default then the status is ok.
writeBeanIfValid return false but the field status is not updated on the screen (not in error).

Is it a correct behaviour ? (field statuses are not refreshed after writebeanIfValid)

MyUI.java.zip


This change is Reviewable

doWriteIfValid don't show updated field status.
For example, an empty required field is not filled by default then the status is ok.
writeBeanIfValid return false but the field status is not updated on the screen (not in error).
@CLAassistant
Copy link

CLAassistant commented Sep 8, 2017

CLA assistant check
All committers have signed the CLA.

@jcgueriaud
Copy link
Contributor Author

I add one comment on my "patch": Binder is a quite hard to understand and to modify.
So I just write something to get the "expected" behaviour which is in my opinon that writeBeanIfValid = validate then writeBean.
So in my opinion, writeBeanIfValid should be rewrite reusing validate function (instead of copy/paste like I do).
I can also call validate function juste after readBean and all the errors will be displayed (before calling writeBean).

Regards,

@tsuoanttila
Copy link
Contributor

a discussion (no related file):
Hi @jcgueriaud !

Thanks for the Pull Request. However, there's another fix to the Binder, that does touch the same part of the Binder and makes more or less the same change. Can you test if the change in #9988 against your UI? If you don't have the time, I'll do it a bit later when I get back to this.


Comments from Reviewable

@jcgueriaud
Copy link
Contributor Author

Hi @tsuoanttila
I've just try your modifications and it does fix my problem.

Really nice patch :)
Thanks !

I close my pull request because it's fixed by #9988

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants