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.3k stars 305 forks source link

fix: parameter binding issue in FluentDesignTheme #1455

Closed lus closed 3 months ago

lus commented 3 months ago

πŸ› Bug Report

I am trying to realize a simple panel to manage the page appearance, just like on the demo page. To do this, I have created the two parameters (mode and officeColor) in the panel component's code area and bound them to their corresponding FluentDesignTheme and FluentSelect components. This works when not including any more code in the panel component. As soon as any state is given to the component (using its Content parameter) or even if OnInitializedAsync is implemented, the parameters will be ignored and are reset everytime the panel is opened.

πŸ’» Repro or Code Sample

Working example:

@implements IDialogContentComponent

<FluentDesignTheme StorageName="theme" @bind-Mode="@_mode" @bind-OfficeColor="@_color"/>
<FluentSelect Label="Theme" Items="@(Enum.GetValues<DesignThemeModes>())" @bind-SelectedOption="@_mode"></FluentSelect>
<div style="height: 20px"></div>
<FluentSelect Label="Color"
              Items="@(Enum.GetValues<OfficeColor>().Select(i => (OfficeColor?)i))"
              @bind-SelectedOption="@_color">
    <OptionTemplate>
        <FluentStack>
            <FluentIcon Value="@(new Icons.Filled.Size20.RectangleLandscape())"
                        Color="Color.Custom"
                        CustomColor="@(context.ToAttributeValue() != "default" ? context.ToAttributeValue() : "#036ac4")"/>
            <FluentLabel>@context</FluentLabel>
        </FluentStack>
    </OptionTemplate>
</FluentSelect>

@code
{
    private DesignThemeModes _mode;

    private OfficeColor? _color;
}

Failing example:

@using App.Ui.State
@implements IDialogContentComponent
@inject AppState State

<FluentDesignTheme StorageName="theme" @bind-Mode="@_mode" @bind-OfficeColor="@_color"/>
<FluentSelect Label="Theme" Items="@(Enum.GetValues<DesignThemeModes>())" @bind-SelectedOption="@_mode"></FluentSelect>
<div style="height: 20px"></div>
<FluentSelect Label="Color"
              Items="@(Enum.GetValues<OfficeColor>().Select(i => (OfficeColor?)i))"
              @bind-SelectedOption="@_color">
    <OptionTemplate>
        <FluentStack>
            <FluentIcon Value="@(new Icons.Filled.Size20.RectangleLandscape())"
                        Color="Color.Custom"
                        CustomColor="@(context.ToAttributeValue() != "default" ? context.ToAttributeValue() : "#036ac4")"/>
            <FluentLabel>@context</FluentLabel>
        </FluentStack>
    </OptionTemplate>
</FluentSelect>
@if (_isDeveloperModeEnabled)
{
    <div style="height: 20px"></div>
    <FluentTextField Label="Server URL Override"></FluentTextField>
}

@code
{
    private DesignThemeModes _mode;

    private OfficeColor? _color;

    private bool _isDeveloperModeEnabled;

    protected override async Task OnInitializedAsync()
    {
        _isDeveloperModeEnabled = await State.IsDeveloperModeEnabledAsync();
    }
}

In the MainLayout.razor I have added <FluentDesignTheme StorageName="theme"/> and the panel is opened using this method:

private async void OpenPageSettingsPanel()
{
    await DialogService.ShowPanelAsync<PageAppearancePanel>(
        new DialogParameters
        {
            Alignment = HorizontalAlignment.Right,
            Title = "Page settings",
            PrimaryAction = "Ok",
            SecondaryAction = null
        });
}

πŸ€” Expected Behavior

I expect the parameters to be bound and loaded correctly.

😯 Current Behavior

As staed above.

πŸ”¦ Context

As stated above.

🌍 Your Environment

vnbaaij commented 3 months ago

I don't understand. Which parameters are ignored/reset?

lus commented 3 months ago

The OfficeColor parameter in particular seems to be ignored in this case. The background color randomizes everytime the panel is (re-)opened and the primaryColor field in the local settings is being set to a (random?) hex code,

vnbaaij commented 3 months ago

I can't replicate that with the code you supplied. Need a more complete example or repo.

A randomizing background (you mean accent?) color must come from somewhere else as nothing is being done with that in the code you posted,

lus commented 3 months ago

I will try to set up a minimal example as the code this is happening in is confidential.

lus commented 3 months ago

Hello again. I have created a minimal example with which I could reproduce the issue. I want to make clear that I don't want to rule out that the issue is due to some fundamental misunderstanding of mine about how the component lifecycle affects things.

The repo:

https://github.com/lus/FluentThemeBugRepro

A video showcasing the whole process:

https://github.com/microsoft/fluentui-blazor/assets/46935044/2b014c8f-97d9-49c5-addd-d1522d2d1942

vnbaaij commented 3 months ago

In your video, the color in storage is not being changed on selecting one from the dropdown. When I run the repro code locally, that does happen. I'm a little lost on what the issue is exactly issue-#1455

lus commented 3 months ago

I also don't fully understand why that happens. Pay attention to my timing in the video as well. The "bug" only occurs when making changes after the Task in OnInitializedAsync has finished as it seems.

vnbaaij commented 3 months ago

And when is that point in the video? What am I seeing then that is different? Sorry, I don't understand what your "bug" is.

lus commented 3 months ago

As you have already said, "the color in storage is not being changed on selecting one from the dropdown". This is the bug I am facing and the reason for this issue. My last comment was just meant as a hint for your attempt on reproducing the issue. I am sorry if I am explaining it a bit confusing, English is not my native language.

vnbaaij commented 3 months ago

No excuse needed. It is not mine either πŸ˜‰

Ok, so now that we have established that the bug is that the changed color is not reflected in storage, how can I replicate that in the code repro you created. As seen in my 'video', it just worked after cloning, building and running it. No changes made whatsoever.

lus commented 3 months ago

Alright, I think I have narrowed it down. Please try the following steps:

  1. Make sure local storage is empty
  2. Open the panel, wait until values are saved to local storage. Don't make any changes
  3. Close the panel and open it again

This provokes the bug on my side.

dvoituron commented 3 months ago

This seems to be due to a default (random) color selection, which is not updated correctly when a delay is applied (1000ms). This will be corrected in the next version.

Is it the correct result? Fix-ThemeDesign

lus commented 3 months ago

This looks promising, thank you very much for looking into it!

vnbaaij commented 3 months ago

Closing this as PR is merged and will be in next release.