microsoft / fluentui-blazor

Microsoft Fluent UI Blazor components library. For use with ASP.NET Core Blazor applications
https://www.fluentui-blazor.net
MIT License
3.66k stars 350 forks source link

[Bug] FluentSlider two-way binding issue #2609

Open LuohuaRain opened 1 week ago

LuohuaRain commented 1 week ago

https://github.com/microsoft/fluentui-blazor/pull/2591#issuecomment-2324344948

While the mouse wheel is just one example, there are other ways to interact with the slider. For instance, how can I reset the slider value? Fluent UI React Web Component supports this functionality.

https://react.fluentui.dev/?path=/docs/components-slider--docs#controlled

import * as React from "react";
import { useId, Button, Label, Slider } from "@fluentui/react-components";
import type { SliderProps } from "@fluentui/react-components";

export const Controlled = () => {
  const id = useId();
  const [sliderValue, setSliderValue] = React.useState(160);
  const onSliderChange: SliderProps["onChange"] = (_, data) =>
    setSliderValue(data.value);
  const resetSlider = () => setSliderValue(0);
  return (
    <>
      <Label htmlFor={id}>
        Control Slider [ Current Value: {sliderValue} ]
      </Label>
      <Slider
        aria-valuetext={`Value is ${sliderValue}`}
        value={sliderValue}
        min={20}
        max={200}
        onChange={onSliderChange}
        id={id}
      />

      <Button onClick={resetSlider}>Reset</Button>
    </>
  );
};

React can do this, so how blazor do this?

<div style="display: flex; flex-direction: column; align-items: flex-start; height: min-content;">
    <label for="rtl-slider" style="margin-left: 8px;">rtl support</label>
    <div dir="rtl" style="width: 100%;">
        <FluentSlider id="rtl-slider" Min="0" Max="100" Step="10" @bind-Value=value2>
            <FluentSliderLabel Position="10">10</FluentSliderLabel>
            <FluentSliderLabel Position="20">20</FluentSliderLabel>
            <FluentSliderLabel Position="40">40</FluentSliderLabel>
            <FluentSliderLabel Position="60">60</FluentSliderLabel>
            <FluentSliderLabel Position="80">80</FluentSliderLabel>
        </FluentSlider>

        <FluentButton OnClick="() => value2 = 0 ">Reset</FluentButton>  @* THIS LINE IMPORTANT *@
    </div>
</div>

🤔 Expected Behavior

I change the value by clicking/dragging, and after I want to reset, then I click Reset and the FluentSlider Value should be 0. That's what true two-way data binding means

😯 Current Behavior

Currently, the value does not reset.

vnbaaij commented 1 week ago

The fact that the React component does that, says absolutely nothing about what the web component can do. I think we have an example somewhere. I'll get back on that

LuohuaRain commented 1 week ago

The fact that the React component does that, says absolutely nothing about what the web component can do.

Yes sure!

The example with the React component isn't quite relevant since it doesn't appear to be a Web Component. However, the FAST Element can change the Slider's position by modifying the value property. Wouldn't this example be more convincing?

<!DOCTYPE html>
<html lang="en">

<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <script type="module"
        src="https://cdn.jsdelivr.net/npm/@microsoft/fast-components/dist/fast-components.min.js"></script>
    <title>Document</title>
</head>

<body>
    <fast-slider id="slider" min="0" max="100" step="10" value="70">
        <fast-slider-label position="0">
            0
        </fast-slider-label>
        <fast-slider-label position="10">
            10
        </fast-slider-label>
        <fast-slider-label position="90">
            90
        </fast-slider-label>
        <fast-slider-label position="100">
            100
        </fast-slider-label>
    </fast-slider>
    <fast-button id="resetButton">Reset</fast-button>

    <script>
        document.getElementById('resetButton').addEventListener('click', function () {
            document.getElementById('slider').value = 0; 
        });
    </script>
</body>

</html>

I also tested Fluent UI WebComponent

<div dir="rtl" style="width: 100%;"><!--!--><!--!--><!--!-->
    <fluent-slider id="test" class="horizontal" min="0" max="100" step="10" value="10" role="slider" tabindex="0"
        aria-valuenow="10" aria-valuemin="0" aria-valuemax="100" aria-orientation="horizontal"
        _bl_21a80d11-9111-44ad-b862-7aecd6bac701="" current-value="10" orientation="horizontal"
        mode="single-value"><!--!--><fluent-slider-label id="f7018d0f2" position="10" class="horizontal"
            _bl_60d904da-9a3a-43f2-b86d-69bc9963e25f="" aria-disabled="false">10</fluent-slider-label><!--!-->
        <!--!--><fluent-slider-label id="f314aa7bf" position="20" class="horizontal"
            _bl_61752e38-1d0b-4047-a927-4fe2288a2b4c="" aria-disabled="false">20</fluent-slider-label><!--!-->
        <!--!--><fluent-slider-label id="f4fd768" position="40" class="horizontal"
            _bl_c76d2674-3c54-4257-92ff-5c0ab457b5d6="" aria-disabled="false">40</fluent-slider-label><!--!-->
        <!--!--><fluent-slider-label id="f48b121d3" position="60" class="horizontal"
            _bl_b3317bc3-8152-4be7-98bd-93084b2331b9="" aria-disabled="false">60</fluent-slider-label><!--!-->
        <!--!--><fluent-slider-label id="f457aedb7" position="80" class="horizontal"
            _bl_582433c4-b1a8-495e-b1f7-81e1ca866ba9=""
            aria-disabled="false">80</fluent-slider-label></fluent-slider><!--!-->

    <!--!--><!--!--> <fluent-button type="button" appearance="neutral" class="neutral" b-x1200685t0=""
        _bl_49289cc2-24fb-4050-ab7b-2315f9616f69="" current-value="">Reset</fluent-button>
</div>
document.getElementById('test').value = 10;

And it works.

There's an issue with Blazor's two-way binding for the fluent-slider's value. The value changes, but the UI doesn't refresh. Any idea what's going wrong?

If document.getElementById('test').value = 10 works but directly modifying the value through Blazor's two-way binding doesn't, I suspect that the Web Component or Fluent UI JS might be intercepting related JavaScript modification events. It seems that only changes made via JavaScript trigger updates to other related properties, such as value, aria-valuenow, and current-value

If the above assumption is correct, perhaps we should fix this way?

@namespace Microsoft.FluentUI.AspNetCore.Components
@inherits FluentInputBase<TValue>
@typeparam TValue
@attribute [CascadingTypeParameter(nameof(TValue))]
<FluentInputLabel ForId="@Id" Label="@Label" AriaLabel="@AriaLabel" ChildContent="@LabelTemplate" Required="@Required" style="margin-bottom: calc(var(--design-unit) * 3px);" />
<fluent-slider @ref=Element
               class="@ClassValue"
               style="@StyleValue"
               readonly="@ReadOnly"
               min="@FormatValueAsString(Min)"
               max="@FormatValueAsString(Max)"
               step="@FormatValueAsString(Step)"
               orientation="@Orientation.ToAttributeValue()"
               mode="@Mode.ToAttributeValue()"
               id="@Id"
               value="@FormatValueAsString(Value)"
               current-value="@FormatValueAsString(Value)"  @* ADD THIS LINE *@
               aria-valuenow="@FormatValueAsString(Value)" @* ADD THIS LINE *@
               disabled="@Disabled"
               name=@Name
               required=@Required
               @onsliderchange="@ChangeHandlerAsync">
    @ChildContent
</fluent-slider>
oneolddev commented 1 week ago

@vnbaaij

This issue looks familiar - I submitted #1873 that fixed a redraw issue #1836. It looks like #2287 removed some of that code and may be the reason why this issue is now occurring.

It's been a while but I recall something about how the underlying webcomponent initializes. I can take a look in the morning if you wish.

vnbaaij commented 1 week ago

Yes, it is familiar but it also isn't :)

What I've found so far:

Works when using the Blazor code from @LuohuaRain's opening post. What does not work is using the Slider inside another component that can influence the slider's value. I'm using the DataGrid Typical Usage filter example for this. moving the thumb a couple of times in quick succession throws it into a loop: grid-slider

Not adding the current-value to the slider tag and using the script for earlier PR that Gary (@oneolddev) mentioned has the same result (reset works, grid goes into loop) if we invoke the js function on every render (ie in OnAfterRenderAsync but outside of `firstRender' block):

protected override async Task OnAfterRenderAsync(bool firstRender)
{
    if (firstRender)
    {
        _jsModule ??= await JSRuntime.InvokeAsync<IJSObjectReference>("import", JAVASCRIPT_FILE);
    }
    if (_jsModule is not null)
    {
        await _jsModule!.InvokeVoidAsync("updateSlider", Element);
    }
}

If we move the script call to inside the firstRender block, the data grip filter example works flawlessly. @LuohuaRain's code works for some part not not completely. If you move a slider and then press reset, the value gets set to 0 but the thumb is not updated. If you press reset without moving a slider, it will set the value and the thumb correctly: issue-slider

Now we just need a working combination of these two. I've tried everything I could think of but was not able to find a definitive solution. So Gary, if your offer still stands, please take a look. I crated a draft PR for this (#2612) with updated code to make it a bit easier to run/test the issue.

PS, if we can't get both situations to work reliably, I would say getting the grid situation working ranks higher on my list...

oneolddev commented 1 week ago

I'm looking into this now. I was not aware of the datagrid and slider interaction. ⛏️ ⛏️