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

Slider changes value in bean even when user did not changed it #11890

Closed
SomMeri opened this issue Feb 14, 2020 · 3 comments
Closed

Slider changes value in bean even when user did not changed it #11890

SomMeri opened this issue Feb 14, 2020 · 3 comments
Labels
question Stale Stale bot label

Comments

@SomMeri
Copy link

SomMeri commented Feb 14, 2020

Versions and Systems

Vaadin Framework version: 8.10.0 (It works correctly on older versions)
Browser version: all of them
Web container name and version: all of them

Description of the bug

When I set bean to slider, the slider rounds the value inside the bean. This happens without any user interaction. The bean value should change only if the user moves the slider, the value should not be modified by the binder.setBean(bean) call.

Minimal reproducible example

Slider slider = new Slider();
slider.setResolution(2);

Label label = new Label();

SliderBean bean = new SliderBean();
bean.setValue(100.003);


System.out.println("bean value before: " + bean.getValue());

Binder<SliderBean> binder = new Binder<>();
binder.forField(slider).bind(SliderBean::getValue, SliderBean::setValue);
binder.setBean(bean);
System.out.println("bean value after: " + bean.getValue());

Expected behavior

Bean value remains 100.003 until user moves slider. Console contains:

bean value before: 100.003
bean value after: 100.003

Actual behavior

Bean value is changed to 100.0. Console contains:

bean value before: 100.003
bean value after: 100.0

Further Analysis

This is new problem in 8.10.0, older version does not have the issue. 8.9.4 works correctly.

The issue is likely caused by these new lines in initFieldValue method. The setBean method calls if from here.

@TatuLund
Copy link
Contributor

Your analysis about what is the cause of the change of behavior here is correct.

We did already while ago this change, which addresses some issues like this one

#9000

Also it is worth of mentioning, that as it was slightly behavior altering change the fix was postponed to Vaadin 8.10. The same fix is also in our Vaadin 14 product, so now Binder behavior is the same in both, and this change in Vaadin 8 makes it forward compatible.

So this is not a bug, but intentional behavior change.

I would say that also in this particular case the behavior is now correct.

@SomMeri
Copy link
Author

SomMeri commented Feb 21, 2020

@TatuLund Could it be made easily customizable? The tricks I have to currently do to keep original value in bean are ugly and fragile.

Workaround in case someone else needs this too:

public class InputFieldBinder<M> extends Binder<M> {
        [...]

	@Override
	protected <FIELDVALUE, TARGET> BindingBuilder<M, TARGET> doCreateBinding(HasValue<FIELDVALUE> field, Converter<FIELDVALUE, TARGET> converter, BindingValidationStatusHandler handler) {
		return new KeepInitialValueBindingBuilderImpl<>(this, field, converter, handler);
	}

	/**
	 * See {@link KeepInitialValueBindingBuilderImpl} to see why this is done.
	 */
	private boolean ignoreValueChange = false;

	@Override
	public void setBean(M bean) {
		try {
			ignoreValueChange = true;
			super.setBean(bean);
		} finally {
			ignoreValueChange = false;
		}
	}

	/**
	 * Since vaadin 8.10.0, vaadin changes bean value upon setBean method. This primarily affected the slider -
	 * it rounded value inside the bean. 
	 * 
	 * This is workaround. We do not want to change stored value unless user modified it.
	 * 
	 *
	 * @param <FIELDVALUE>
	 * @param <TARGET>
	 */
	class KeepInitialValueBindingBuilderImpl<FIELDVALUE, TARGET> extends BindingBuilderImpl<M, FIELDVALUE, TARGET> {

		protected KeepInitialValueBindingBuilderImpl(Binder<M> binder,
				HasValue<FIELDVALUE> field,
				Converter<FIELDVALUE, TARGET> converterValidatorChain,
				BindingValidationStatusHandler statusHandler) {
			super(binder, field, converterValidatorChain, statusHandler);
		}

		@Override
		public Binding<M, TARGET> bind(ValueProvider<M, TARGET> getter, Setter<M, TARGET> setter) {
			return super.bind(getter, (bean, value) -> hackedSetter(bean, value, setter));
		}

		private void hackedSetter(M bean, TARGET value, Setter<M, TARGET> setter) {
			if (!ignoreValueChange) {
				setter.accept(bean, value);
			}
		}

	}
}

@stale
Copy link

stale bot commented Jul 20, 2020

Hello there!

We are sorry that this issue hasn't progressed lately. We are prioritizing issues by severity and the number of customers we expect are experiencing this and haven't gotten around to fix this issue yet.

There are a couple of things you could help to get things rolling on this issue (this is an automated message, so expect that some of these are already in use):

  • Check if the issue is still valid for the latest version. There are dozens of duplicates in our issue tracker, so it is possible that the issue is already tackled. If it appears to be fixed, close the issue, otherwise report to the issue that it is still valid.
  • Provide more details how to reproduce the issue.
  • Explain why it is important to get this issue fixed and politely draw others attention to it e.g. via the forum or social media.
  • Add a reduced test case about the issue, so it is easier for somebody to start working on a solution.
  • Try fixing the issue yourself and create a pull request that contains the test case and/or a fix for it. Handling the pull requests is the top priority for the core team.
  • If the issue is clearly a bug, use the Warranty in your Vaadin subscription to raise its priority.

Thanks again for your contributions! Even though we haven't been able to get this issue fixed, we hope you to report your findings and enhancement ideas in the future too!

@stale stale bot added the Stale Stale bot label label Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Stale Stale bot label
Projects
None yet
Development

No branches or pull requests

2 participants