MudTreeViewItem: Add 'BodyContent' which renders instead of text, end text and end icon.

[rpc-scandinavia]

Description

The 'Content' render fragment replaces the entire tree view item, which mean that you have to handle the toggle button and logic your self.

I needed to use the nice logic of the tree view (loading, expanding/collapsing etc.), but provide a custom body.

I also needed a way to reload the tree view item.

How Has This Been Tested?

Uses it in mu project.

Types 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)

Checklist:

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

[mikes-gh]

This doesn't build and would need tests for the new API added. But before you spend more time on this we would need to see what the new API brings. @tungi52

[rpc-scandinavia]

Interesting with a new API in sight.

I use this in my project, with "MudTreeView" renamed to "MudTreeView1". The compile error was one of the "MudTreeView1" left over.

[codecov]

Codecov Report

Patch coverage: 95.00% and project coverage change: -0.01 ⚠️

Comparison is base (e0afebd) 91.37% compared to head (519b0d6) 91.37%.

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #3831      +/-   ##
==========================================
- Coverage   91.37%   91.37%   -0.01%     
==========================================
  Files         397      397              
  Lines       14919    14929      +10     
==========================================
+ Hits        13632    13641       +9     
- Misses       1287     1288       +1     

Impacted Files	Coverage Δ
...MudBlazor/Components/TreeView/MudTreeView.razor.cs	100.00% <ø> (ø)
...lazor/Components/TreeView/MudTreeViewItem.razor.cs	97.43% <92.85%> (-0.73%)	⬇️
...udBlazor/Components/TreeView/MudTreeViewItem.razor	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[tungi52]

The new BodyContent would be nice, if someone need a fully customizable content for a MudTreeItem. Also I understand why the Reload method is necessary, at the moment there is no way to update children from server. I'm only not sure about the naming, on the other hand it is ok. However ReloadParent seems a bit meaningless to me, can you explain, why it is necessary? Also some tests would be great.

[mikes-gh]

I know this is still in discussion but BodyContent should be ChildContent in order to be consistent with the rest of the library

[rpc-scandinavia]

I know this is still in discussion but BodyContent should be ChildContent in order to be consistent with the rest of the library

I just wanted to make that change, but realised that ChildContent is already used.

[tungi52]

Thanks! For me this is ok. Can you add test(s) and an example to demonstrate the new features?

[rpc-scandinavia]

Thanks! For me this is ok. Can you add test(s) and an example to demonstrate the new features?

I will look at that.

[tungi52]

@rpc-scandinavia This doesn't build for me.

[ScarletKuro]

I didn't see this PR when I was creating this isssue #4503 but it looks like this changes should also solve the problems described in the ticket. I will leave the ticket open tho, until this is merged.

[soend]

Anything else stopping this PR to get accepted and merged? I also need a way to use ServerData with custom MudTreeViewItem content. This PR would solve that issue.

[just-the-benno]

I'm sorry that it took as so long to get back.

@rpc-scandinavia, is there still life in this PR? Can we resurrect it?

[rpc-scandinavia]

is there still life in this PR? Can we resurrect it?

Well, everything worked, the only thing missing was the test - and I don't know how to make the test I want. Anyway, there has been so many changes at your end, that I can't get my GitHub copy to work - which probably mean that I have to delete my copy, fork it again, make all the changes again, and create a new pull request. So much hazzle for around 10 lines of code.

I have been using my solution since the first post, and I think we ended up with something good. There was however this question:

With 3) you can consider if BodyContent should be named BodyContentWithContext for consistency.

I only want to spend more time on this GitHub pull, if you put it into the project.

[rpc-scandinavia]

Deleted my fork, and forked the project again.

[rpc-scandinavia]

Ok @henon, I corrected the spelling errors and added some simple tests.

[rpc-scandinavia]

@henon, another thing in MudTreeViewItem.razor.cs, I had to replace all:

StateHasChanged();

With:

InvokeAsync(() => StateHasChanged());

Because of:

Unhandled exception. System.InvalidOperationException: The current thread is not associated with the Dispatcher. Use InvokeAsync() to switch execution to the Dispatcher when triggering rendering or component state.

Do you think that should be done in this PR ?

[ScarletKuro]

I wonder if it's possible to test the new Reload method that was introduced.

another thing in MudTreeViewItem.razor.cs, I had to replace all:

Can you be more specific where, there are many of them
But if the method is Task, i would propose await InvokeAsync(StateHasChanged);

FYI: I think github it dead and you can't push rn.

[ScarletKuro]

Not related to this PR, but why is TryInvokeServerLoadFunc an async void instead of async Task. I see no reason for it and it's always called from a Task and you would be able to await it.
Why _childItems.ForEach(async c => await c.SelectItem(value, source)); (void async lambda) instead of

foeach (var item in _childItems){
    await item.SelectItem(value, source)
}

I recommend to avoid async void unless totally necessary i.e. in event handler, for example.

I kinda want to propose to make Reload ->ReloadAsync with an Task since MudTree has async semantics. the MudTreeRoot.ServerData is async and it would allow me to make a PR with some inner changes to improve the component without breaking the public API.

[henon]

Yes and of course the ReloadAsync function can also be tested easily. I.e. by changin the server data before calling it and checking that the changed data is reflected in the tree.

[henon]

Do you think that should be done in this PR ?

Only if you are sure it is necessary. The first question to ask is why were they called from the wrong thread. What caused this?

[ScarletKuro]

Only if you are sure it is necessary. The first question to ask is why were they called from the wrong thread. What caused this?

It's only my guess, but maybe he modified some state in a test, like changing value of [parameter] property.
Those should be called like this in a test
await comp.InvokeAsync(() => item.Instance.Disabled = true);
await comp.InvokeAsync(() => comp.Instance.SelectFirst());

[rpc-scandinavia]

Not related to this PR, but why is TryInvokeServerLoadFunc an async void instead of async Task.

I saw that as well, I am working on another PR where I call TryInvokeServerLoadFunc and noticed that. So I changed it.

And my InvokeAsync are awaited.

[ScarletKuro]

I saw that as well

Ok, but lets keep it as is in this PR.

Please, change void Reload to void ReloadAsync and add test for this method like henon suggested:

Yes and of course the ReloadAsync function can also be tested easily. I.e. by changin the server data before calling it and checking that the changed data is reflected in the tree.

And then I guess we are good to go and merge this PR.

[ScarletKuro]

If you rebase to latest changes there is new interface now and you can do something like this
((IMudStateHasChanged)MudTreeRoot).StateHasChanged() and remove the CallStateHasChanged

[rpc-scandinavia]

@ScarletKuro, I have made the async and IMudStateHasChanged changes.

[ScarletKuro]

lgtm

[henon]

Thanks for your contribution @rpc-scandinavia !