MudDataGrid: Correctly render initially expanded rows

[Anu6is]

Description

The current implementation adds the initially expanded items to the OpenHierarchies collection of the CellContext. This however, doesn't cause the DataGrid to re-render. The update, instead, calls ToggleHierarchyVisibilityForItemAsync for each item, which executes a method on the DataGrid and forces a re-render

Resolves #10104

How Has This Been Tested?

Updated existing unit test to confirm that that row is actually expanded in the markdown.

Type of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation (fix or improvement to the website or code docs)

Checklist

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.18%. Comparing base (85b3d96) to head (81cdeef).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev   #10133      +/-   ##
==========================================
- Coverage   91.18%   91.18%   -0.01%     
==========================================
  Files         411      411              
  Lines       12483    12480       -3     
  Branches     2432     2431       -1     
==========================================
- Hits        11383    11380       -3     
  Misses        556      556              
  Partials      544      544              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Anu6is]

Was just about to fix it.... that's what you get for trying quick edits on your phone xD

[ScarletKuro]

Thanks!

[ScarletKuro]

A quick question, does this fixes #9681 as well or not?

upd: Nvm, i think it doesn't

[Anu6is]

It does not.
I'm honestly not fully convinced about what that issue is asking for either

[ScarletKuro]

I'm honestly not fully convinced about what that issue is asking for either

The problem is that when you add new items to the data grid on the fly, the ToggleHierarchyVisibilityForItemAsync method is no longer executed because _finishedInitialExpanded is already set to true after OnAfterRenderAsync (There is only one call of it in the HierarchyColumn during the data grid's lifetime). This means that newly added items, even if they meet the InitiallyExpandedFunc conditions, will not be expanded. I hope that makes sense.

I know how to fix it in a reactive way. You can implement a pub/sub using either IAsyncEnumerable or a custom primitive. You would subscribe to it in the background during OnAfterRenderAsync, and when the CellTemplate of the HierarchyColumn is accessed, you can add the new item to the pub/sub, and it will be processed(requires to implement IEquatable for the CellContext, as a HashSet cannot effectively track already subscribed contexts with the default runtime equality implementation) This works, but I might be over-engineering it as a backend dev. I’m not going to bother submitting it, there should be a simpler way, but I don't have enough patience to proceed it.

[Anu6is]

Ok, maybe it was my misunderstanding of the initially expanded feature. I took it to mean when the grid is initially loaded/rendered the specified rows should be expanded (which it currently does). From your explanation though, it's not just when the grid is initially loaded but whenever data is added it needs to check if the newly added rows should render expanded or not.

[DueData]

Hello, Is this fix in progress? Seems like it's still an outstanding issue - https://try.mudblazor.com/snippet/mkwIlcQNCoZdniOV

[Anu6is]

Hello, Is this fix in progress? Seems like it's still an outstanding issue - https://try.mudblazor.com/snippet/mkwIlcQNCoZdniOV

This fix was implemented in v8. TryMud uses v7. You'd need to test using a v8 implementation to confirm.

[DueData]

I tried v8.0.0-preview.6 locally and still seeing the issue.

[ScarletKuro]

I think the issue here is:

private IEnumerable<Element> Elements = new List<Element>();

protected override async Task OnInitializedAsync()
{
    Elements = await httpClient.GetFromJsonAsync<List<Element>>("webapi/periodictable");
}

You have two states: initially, the DataGrid has empty elements, and then, after receiving the data, it transitions to the second state.

Unfortunately, the InitiallyExpandedFunc is only invoked once during the DataGrid's lifecycle. As a result, it will only work properly if you implement something like this:
https://try.mudblazor.com/snippet/QkQpkPPcsozRJNed

[ScarletKuro]

I guess accepting InitiallyExpandedFunc was a mistake, as it had a poor design from the start. I don't think the expansion logic should be handled at the HierarchyColumn level, like in OnAfterRenderAsync. Instead, it should be implemented somewhere within the MudDataGrid itself, when it's listing the rows. Alternatively, the datagrid could have events that trigger when the grid state changes, such as filtering, sorting, or changing pages, to allow the InitiallyExpandedFunc to be reevaluated on those changes internally.

[DueData]

I guess accepting InitiallyExpandedFunc was a mistake, as it had a poor design from the start. I don't think the expansion logic should be handled at the HierarchyColumn level, like in OnAfterRenderAsync. Instead, it should be implemented somewhere within the MudDataGrid itself, when it's listing the rows. Alternatively, the datagrid could have events that trigger when the grid state changes, such as filtering, sorting, or changing pages, to allow the InitiallyExpandedFunc to be reevaluated on those changes internally.

Thank you for the response. Is there already work underway to implement this?

[Anu6is]

I don't believe this is being worked on at the moment