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.23k stars 294 forks source link

fix: FluentTabs in Virtualize component - An item with the same key has already been added. #2005

Closed mpcham closed 1 week ago

mpcham commented 2 weeks ago

๐Ÿ› Bug Report

I have a html table where rows are wrapped in the Virtualize component. Each row has a few tabs in one of the columns. The Id of the tab is manually generated using the rows unique key. e.g. "isolations-tab-anchorId-1234" or "efra-tab-anchorId-1234". The Id is also used as the AnchorId for the FluentToolTip component which is specific to each tab. Using the FluentToolTip is the only reason I manually generate the FluentTab Id's.

Each Row can have 0 or 5 tabs. Between 1 - 5 tabs, 1st tab is always the "Hide" tab, followed by 1-4 others. The "Hide" FluentTab (i.e. the 1st tab) never throws the error and has the Id set the same way as the other tabs. The "Isolations" FluentTab always seems to be the one that reports the error (as seen in the image below) yet it's usually the 2nd tab.

This issue does not occur when the FluentTab Id's are not set, i.e. automatically generated. This issue also does not occur when used outside of the Virtualize component.

I have several other FluentToolTip components tied to other html elements which also have manually generated AnchorId's but these never cause an issue.

๐Ÿ’ป Repro or Code Sample

This is a snippet of how the FluentTabs > FluentTab & FluentToolTip part of each row is setup. As mentioned previously, each FluentTab.Id is unique to the Row and Tab so there is no way for duplicate Id's to exist.

image

๐Ÿค” Expected Behavior

When scrolling down the table, then back up and the virtualize component reloads rows previously rendered this should be handled normally and not throw an error.

๐Ÿ˜ฏ Current Behavior

When scrolling down the table and triggering the render of the next set of rows, defined by the OverScanCount on the Virtualize component, then scrolling back up to render the first set of rows, the following error is thrown.

image

๐Ÿ’ Possible Solution

Looks specific to user generated Id's for the FluentTabs component when used within the Virtualize component.

Happy to jump on a call to demo the issue. It's a complex table so a lot going on but I think it should handle this scenario.

๐Ÿ”ฆ Context

I've tried not using the Virtualize component but it's necessary as the user can return 3 rows or thousands. At present, I've removed use of the FluentToolTip component and therefore not specified the Id's on each of the FluentTabs on the page.

The FluentToolTip component is useful in this scenario because you can use abbreviations with a description hidden within the tooltip providing a neater display yet informative when the user needs it.

๐ŸŒ Your Environment

Exists in

Running latest .NET at time of this issue report: .NET 8.0.4 & 8.0.3 Fluent UI Blazor library 4.7.1 & 4.7.0

vnbaaij commented 2 weeks ago

What is probably happening is that on scrolling the virtualized list, tabs are being re-rendered. It goes to here:

protected override void OnInitialized()
    {
        Index = Owner!.RegisterTab(this);
    }

which calls

internal int RegisterTab(FluentTab tab)
    {
        _tabs.Add(tab.Id!, tab);
        return _tabs.Count - 1;
    }

I suspect w can solve it by changing the _tabs.Add to _tabs.TryAdd but I need to have a situation I can test against. Is there a repo or example code you can share to test (as we request in the issue template)?

vnbaaij commented 1 week ago

The fix (not sure if it works in your case as we had no code to test against) is in v4.7.2 release.

Please test and let us know if it is resolved. Thanks.

mpcham commented 1 week ago

Issue is confirmed as fixed in 4.7.2 as you suggested. Apologies for no Repro to help you guys out testing. I didn't get chance to create one from scratch. Luckily it was a straight forward error.

Thanks for your prompt support!