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

fix: desync of FluentNumberField and FluentSlider in 4.10.4 #2948

Open
arazni opened this issue Nov 18, 2024 · 5 comments
Open

fix: desync of FluentNumberField and FluentSlider in 4.10.4 #2948

arazni opened this issue Nov 18, 2024 · 5 comments
Labels
status:in-progress Work is in progress

Comments

@arazni
Copy link

arazni commented Nov 18, 2024

🐛 Bug Report

Seeming regressions of #1531 and #1450

There are 2 Problems

FluentNumberField

This test demonstrates that the FluentNumberField's two-way data binding is broken in version 4.10.4. This appears to be a regression.

In my web app, I have a component that takes a numeric input and divies a portion to a wallet and a portion to a stash. After a number is selected, a button triggers a command and this command also resets the value of the numeric field to 0.

Unfortunately, the numeric field does not respect the behavior of being reset to 0 by the button command.

Go ahead and select a number like 2, which will enable the reset button. The expected behavior is for the reset button to set the value back to 0, and for the number input field to reflect that value.

You can see in the console logs that the data model's value is reset to 0, however the number field's value is still non-zero both visibly and under the hood. This implies that it isn't respecting the binding.

FluentSlider

This test demonstrates that the FluentSlider's two-way data binding can desync from its bound value in version 4.10.4. This may be a regression.

While you can see the behavior working for the Reset button, the behavior does not always work for the "Change by 2" button.

Go ahead and set the slider to 3, which will enable the reset button (only enabled at 3). Reset. You see that it works.

Go ahead and set the slider to 1, then keep pressing the "Change by 2" button. In addition to the visual weirdness, by checking the console logs, you will see that it desyncs from the actual value in the data.

When I run it, the console logs always have an accurate data value for the "Change by 2" button's usage. The underlying slider value on blazor side is one step behind. And the visual only updates every 3 clicks of the button.

A second test scenario is: 1. Refresh the page. 2. Set the slider to 1. 3. Click "Change by 2". 4. Click Reset. This will break the visual synchronization with the "Reset" button, which is otherwise usually working correctly.

💻 Repro or Code Sample

https://github.com/arazni/blazor-fluent-ui-slider-bug/tree/main

🤔 Expected Behavior

When changing a 2-way bound value outside of the component, the visual and component's value should stay synchronized.

😯 Current Behavior

There are scenarios where the component does not match the underlying data. See the repro steps on the repo.

💁 Possible Solution

🔦 Context

Same as #1531 and #1450, I have buttons that change values nearby fields and sliders that take those changes. See https://arazni.github.io/blades-in-the-sheets/demo for context. You may need to enable sliders over checkboxes in the accessibility tab. Then go back to the demo page, The fund section with the pencil icon enabled is where some of these issues come up.

🌍 Your Environment

  • At least Windows
  • At least Mozilla FireFox
  • .NET 9 and Fluent UI Blazor library Version 4.10.4
@microsoft-github-policy-service microsoft-github-policy-service bot added the triage New issue. Needs to be looked at label Nov 18, 2024
@oneolddev
Copy link
Contributor

@vnbaaij @arazni

I'll take a look at the slider issue reported. I previously corrected another regression with #2665.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 20, 2024

Hi @arazni,

There are 2 Problems

Then there are two issues...Please don't mix multiple issues in one the next time.

We'll wait and see what @oneolddev comes up with for the slider. Wrt the NumberField, there is no way I get it to work for both this situation and #2803 at the same time. As I think that last one is more important than being able to manipulate the value from the outside, we won't be able to fix your issue at this time.

@vnbaaij vnbaaij added status:in-progress Work is in progress and removed triage New issue. Needs to be looked at labels Nov 20, 2024
@oneolddev
Copy link
Contributor

@arazni,

I can confirm the issue you found with FluentSlider and believe I have a solution. I just need to be sure that I don't cause another regresssion.

Many of the issues that have been filed regarding FluentSlider is due to the confusion regarding the binding of the underlying fluent-slider web component. The underlying web component doesn't provide a complete two-way binding. The initial value for fluent-slider is only bound on creation. Changes to the fluent-slider value due to movement of the slider is bound to the FluentSlider value which is what is expected. But changing the value of fluent-slider after creation through the initial binding does not work.

This is the reason for the existence of the javascript for FluentSlider. It provides the appearance of a full two-way bindng. In this case, one of the state transitions is not correctly accounted for.

oneolddev added a commit to oneolddev/fluentui-blazor that referenced this issue Dec 21, 2024
@oneolddev
Copy link
Contributor

@arazni

Sorry for the delay, I got very busy and didn't have a chance to get back to this till a couple of days ago. I've just submitted a PR that should resolve your issues.

Please let me know if this works for you.

@arazni
Copy link
Author

arazni commented Dec 22, 2024

Looking forward to testing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:in-progress Work is in progress
Projects
None yet
Development

No branches or pull requests

3 participants