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: NavMenu Expanded or ExpandContent does not update when inside FluentLayout #1550

Closed joriverm closed 3 months ago

joriverm commented 3 months ago

πŸ› Bug Report

Hi, My Client and I are currently developing a application in blazor using FluentUI and i have started to create a MainLayout for all future blazor applications that the client will make. I started by using a FluentNavMenu to handle navigations. the client wanted to have the Icon to expand or collapse the navigation be different from the default 3 stripes. i implemented this using the FluentNavMenu's ExpanderContent, which has a FluentIcon depending on the FluentNavMenu's Expanded property. However, the icon does not seem to update when the FluentNavMenu is inside a FluentLayout component. when i remove the FluentLayout, and the div that contains the whole layout, the code seems to work. However, the NavMenuLink's do update, and their text is hidden when collapsing.

I suspect this has to do with #1387 but im not really sure, as the website is a razor library that is started by a wasm client and i think the divs should have no effect there?

πŸ’» Repro or Code Sample

in the Story book i changed NavMenuCollapsible.razor to have the following :

<FluentNavMenu Width="150" Collapsible="true" Title="Collapsible demo" ExpandedChanged=@ValueChanged>
    <ExpanderContent>
        <FluentIcon Icon=Icon Width="20px" Value=@(Expanded ? new Icons.Regular.Size20.ChevronDoubleLeft() : new Icons.Regular.Size20.ChevronDoubleRight()) />
    </ExpanderContent>
    <ChildContent>
        <FluentNavLink Icon="@(new Icons.Regular.Size24.Home())" Tooltip="Item 1 tooltip">Item 1</FluentNavLink>
        <FluentNavLink Icon="@(new Icons.Regular.Size24.Cloud())" Tooltip="Item 2 tooltip">Item 2</FluentNavLink>
        <FluentNavGroup Title="Item 3" Tooltip="Item 3 tooltip" Icon="@(new Icons.Regular.Size24.EarthLeaf())">
            <FluentNavLink Icon="@(new Icons.Regular.Size24.LeafOne())" Tooltip="Item 3.1 tooltip">Item 3.1</FluentNavLink>
            <FluentNavLink Icon="@(new Icons.Regular.Size24.LeafTwo())" Tooltip="Item 3.2 tooltip">Item 3.2</FluentNavLink>
        </FluentNavGroup>
        <FluentNavLink Icon="@(new Icons.Regular.Size24.CalendarAgenda())" Disabled="true" Href="https://microsoft.com" Tooltip="Item 4 tooltip1">Item 4</FluentNavLink>
    </ChildContent>
</FluentNavMenu>

this works as expected. However, when i add it to the DemoNavMenu.razor, it breaks and i always get the ChevronDoubleLeft as icon

<FluentNavMenu Id="main-menu" Title="Main menu" Expanded=@Expanded Collapsible>
    <ExpanderContent>
        <FluentIcon Icon=Icon Width="20px" Value=@(Expanded ? new Icons.Regular.Size20.ChevronDoubleLeft() : new Icons.Regular.Size20.ChevronDoubleRight()) />
    </ExpanderContent>
    <ChildContent>
    ...
    </ChildContent>
    ...

ive also tried to not bind to Expanded and instead set ExpandedChanged to do the following : ExpandedChanged=@(v => {NavMenuExpanded = v; StateHasChanged();}

this didn't change anything...

πŸ€” Expected Behavior

the icons to change in both the layout, as the demo page

😯 Current Behavior

demo page : icon changes when toggling collapse layout : icon does not change when collapsing

πŸ’ Possible Solution

Im willing to help fix this, as i can reproduce it easily, however, im not sure where the issue comes from. a div shouldn't stop it from executing statehaschanged correctly, right?

πŸ”¦ Context

Creating an application Layout with icons, depending on the navmenu's collapsed state

🌍 Your Environment

NickHirras commented 3 months ago

I faced maybe the same issue in my blazor-wasm project this past week. You can also just add something like

<h1>@expanded</h1>

somewhere in the NavMenu.razor file, and you can visually see the value doesn't appear to update when expanding/collapsing the menu. We worked around by using some CSS to hide the elements when menu was collapsed:

<div class="navmenu-header">Testing</div>
#main-menu.collapsed > .navmenu-header {
    display: none;
}

But would rather be able to have logic based on that expanded bool property.

joriverm commented 3 months ago

ah yes, i could've looked at the css to hide things. that could work, but ye i'd prefer to use the Expanded property as well :)

c0g1t8 commented 3 months ago

@joriverm

Are you encountering this in a Blazor WebAssmbly application? I just wanted to confirm it's not Blazor Web App.

NickHirras commented 3 months ago

@c0g1t8 for me I'm seeing this in a blazor wasm app but not sure about @joriverm

c0g1t8 commented 3 months ago

I think this may also be fixed in PR #1560. It prevents the attachment of the javascript listeners to the collapse/expand element when hosted in a standalone Blazor WASM application. 🀞

joriverm commented 3 months ago

@c0g1t8 & @NickHirras : we use a wasm application, but the fluentUI storybook is a WebApp. maybe it needs to be tested in the storybook with the fix included. today i am stuck in meetings, but we should maybe retest it using the changes i mentioned above? :)

vnbaaij commented 3 months ago

@joriverm what is the storybook you are referring to?

joriverm commented 3 months ago

@joriverm what is the storybook you are referring to?

excuse me, i meant the demo app of fluent UI :D

by the looks of it, it works in the demo app now. but in our wasm app it doesn't so it probably has to do with the script only working in server rendering i assume? :)

vnbaaij commented 3 months ago

Ok, the live demo site is WASM as well, not a WebApp (served by Azure Static Web Apps though)

joriverm commented 3 months ago

Ok, the live demo site is WASM as well, not a WebApp (served by Azure Static Web Apps though)

ah, hmm i might indeed have looked at the wrong project. i will need to investigate further why it doesn't work locally in the project then!

joriverm commented 3 months ago

ok, update : it works in the wasm project the client and i are making. however, i had to change

<FluentNavMenu Class=@NavMenuClass Title=@NavMenuTitle Collapsible Expanded=@NavMenuExpanded >
    <ExpanderContent>
        <FluentIcon Icon=Icon Width="20px" Value=@(NavMenuExpanded ? new Icons.Regular.Size20.ChevronDoubleLeft() : new Icons.Regular.Size20.ChevronDoubleRight()) />
    </ExpanderContent>
    ...

to

<FluentNavMenu Class=@NavMenuClass Title=@NavMenuTitle Collapsible ExpandedChanged="(v) => {NavMenuExpanded = v; StateHasChanged();}">
    <ExpanderContent>
        <FluentIcon Icon=Icon Width="20px" Value=@(NavMenuExpanded ? new Icons.Regular.Size20.ChevronDoubleLeft() : new Icons.Regular.Size20.ChevronDoubleRight()) />
    </ExpanderContent>
    ...

i assume the binding is not being updated, or state hasn't been detected as changed?

vnbaaij commented 3 months ago

So this one can be closed now?

joriverm commented 3 months ago

ill close this because its : A) fixed for the client's app B) so we can no longer think of it.

i don't know why the direct binding to expanded doesn't work, but its probably something silly. that said, it works now! :)