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

Add a method to Binder to check if a specific Binding changed #17395

Closed
felix-office opened this issue Aug 8, 2023 · 7 comments · Fixed by #17861
Closed

Add a method to Binder to check if a specific Binding changed #17395

felix-office opened this issue Aug 8, 2023 · 7 comments · Fixed by #17861

Comments

@felix-office
Copy link

felix-office commented Aug 8, 2023

Describe your motivation

Inside our binder, we use a custom field for address input. If the values in it change, I need to persist the new address object before I can persist the binder bean. So I need to be able to find out if the address field has changed. Binder#hasChanged() only tells me if any field of the binder has changed.

Describe the solution you'd like

The binder already has a set of changed bindings https://github.com/vaadin/framework/blob/master/server/src/main/java/com/vaadin/data/Binder.java#L1536.
We can use this to add a method hasChanges(Binding<BEAN, ?> binding).

In the perfect word it would also remove the "change state" if the value is changed back to the original one but I don't see that this is currently a feature of the Binder.

Describe alternatives you've considered

Building the logic myself using the value change listener.

Additional context

Is there anything else you can add about the proposal?

@benedikt-roth
Copy link
Contributor

Hello, is this still needed? I would like to contribute to the project and can have a look at it.

@knoobie
Copy link
Contributor

knoobie commented Oct 10, 2023

Still up to grab

@benedikt-roth
Copy link
Contributor

Okay, can you please assign this to me?

One more clarification: Is this change supposed to be done in flow or the framework repo? I assume that is to be done in flow.

Best,
Beneidkt

@knoobie
Copy link
Contributor

knoobie commented Oct 13, 2023

@mshabarov sounds like you should answer this :)

@mshabarov
Copy link
Contributor

mshabarov commented Oct 13, 2023

@benedikt-roth thank you very much for willing contribute! 👍
Yes, this is still good to have in Flow.
This feature should be contributed to Vaadin Flow repo, to the Binder class (not necessary only there, but this is a main API).

Vaadin Flow team will review it as soon as we can. Feel free to post even a draft PR or incomplete PR, will try to help.

@knoobie thanks for pinging me, I missed the notification for this.

benedikt-roth added a commit to benedikt-roth/flow that referenced this issue Oct 19, 2023
Bindings. Overloads the existing Binder#hasChanges
method.

GH issue vaadin#17395
@benedikt-roth
Copy link
Contributor

☝🏼 this is my proposal. Let me know what you think.

I was wondering whether there needs to be some additional check to see whether the provided binding is associated with the binder at all. What do you think?

@mcollovati mcollovati moved this to ⚒️ In progress in Vaadin Flow ongoing work (Vaadin 10+) Oct 19, 2023
mcollovati pushed a commit that referenced this issue Oct 20, 2023
Allows to check if a specific binding has uncommitted changes.
Overloads the existing Binder#hasChanges method.

Fixes #17395
@github-project-automation github-project-automation bot moved this from ⚒️ In progress to Done in Vaadin Flow ongoing work (Vaadin 10+) Oct 20, 2023
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.0.alpha1 and is also targeting the upcoming stable 24.3.0 version.

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