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.49k stars 322 forks source link

fix: (Regression) ExpandedChanged binding in FluentNavGroup no longer firing #2289

Closed teyc closed 1 week ago

teyc commented 1 week ago

🐛 Bug Report

Versions affected: 4.8.0, 4.81 Version not affected: 4.7.2

💻 Repro or Code Sample

   <nav class="sitenav">
    <FluentNavMenu Id="bug-menu">
        <FluentNavGroup @bind-Expanded="expanded">
            <TitleTemplate>
                Sample Title @expanded
            </TitleTemplate>
        </FluentNavGroup>
    </FluentNavMenu>
    </nav>
</div>

@code {
    private bool expanded = true;
}

🤔 Expected Behavior

Clicking should toggle value image image

😯 Current Behavior

Clicking doesn't toggle value image image

💁 Possible Solution

Untested https://github.com/microsoft/fluentui-blazor/compare/dev...teyc:fix/2289-ExpandedChanged?expand=1 Related to https://github.com/microsoft/fluentui-blazor/commit/d0a0a4edf67dd98b0a805e265e5e10cc589a4b6f

🔦 Context

We use the Expanded state to virtualise child content, as we have a lot of data.

🌍 Your Environment

Versions affected: 4.8.0, 4.81 Version not affected: 4.7.2

vnbaaij commented 1 week ago

Is this Server or WebAssembly?

vnbaaij commented 1 week ago

This is probably going to be difficult to replicate. Can you supply a zip of a project or a repo to work against?

teyc commented 1 week ago

Issue-2289.zip see possible fix. I couldn't build fluentui-blazor https://github.com/microsoft/fluentui-blazor/compare/dev...teyc:fix/2289-ExpandedChanged?expand=1 This regression is caused by Expanded property being set too early, and causes the change detection to not fire the value has changed.

ASchweers commented 1 week ago

Same issue here. But only the first FluentNavGroup-Element is affected by this (in the demo-app it's the "Getting Started"-NavGroup).

vnbaaij commented 1 week ago

Issue-2289.zip see possible fix. I couldn't build fluentui-blazor https://github.com/microsoft/fluentui-blazor/compare/dev...teyc:fix/2289-ExpandedChanged?expand=1 This regression is caused by Expanded property being set too early, and causes the change detection to not fire the value has changed.

Your example code is faulty. The NavMenu component is missing and the project is using SSR so Expanded will never be set (needs interactivity)

teyc commented 1 week ago

Thank you for looking at this so quickly! I had accidentally removed too much code when I cleaned up the sample.

Please have a look at Home.razor and observe the Expanded = when you switch from Version 4.8.0 to Version 4.7.2 Issue-2289.zip

vnbaaij commented 1 week ago

Ok, we are going back to the 4.7.2 code for the next release. With that this issue will be solved and fixed.

We made a change to accommodate working with prerender: false in the Routes rendermode. When prerendering is false, the page script component gets triggered and expanding/collapsing is actually being done by script instead of by the interactive code. The problem you are seeing is a side-effect of that in combination with the changes we made.