Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Workaround for metrics tree view selection issue #1283

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

tlmii
Copy link
Member

@tlmii tlmii commented Dec 7, 2023

This is a workaround for #1262. We should probably be using @key here anyway because of the loop(s). But using it in this way also has the effect of causing blazor to re-render the elements which clears the selection when we want it to.

This doesn't solve the fact that setting _selectedTreeItem doesn't have the effect we want, just for our use case at the moment this seems sufficient to get by.

@@ -36,7 +36,7 @@
<FluentTreeItem Class="metrics-tree-item" Text="@meterGroup.Key.MeterName" Data="@meterGroup.Key" title="@meterGroup.Key.MeterName" InitiallyExpanded="true" InitiallySelected="@(_selectedInstrument == null && meterGroup.Key.MeterName == _selectedMeter?.MeterName)">
Copy link
Member

@JamesNK JamesNK Dec 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<FluentTreeItem Class="metrics-tree-item" Text="@meterGroup.Key.MeterName" Data="@meterGroup.Key" title="@meterGroup.Key.MeterName" InitiallyExpanded="true" InitiallySelected="@(_selectedInstrument == null && meterGroup.Key.MeterName == _selectedMeter?.MeterName)">
<FluentTreeItem @key="@meterGroup.Key" Class="metrics-tree-item" Text="@meterGroup.Key.MeterName" Data="@meterGroup.Key" title="@meterGroup.Key.MeterName" InitiallyExpanded="true" InitiallySelected="@(_selectedInstrument == null && meterGroup.Key.MeterName == _selectedMeter?.MeterName)">

I tested and it is required to stop the first level tree items having the same selection issue.

Unfortunately it introduced a rendering quirk where second level items jump slightly when loaded:

metrics-tree-jump

The same thing happened previously when a resource is first selected and the tree is displayed. It wasn't as obvious because it only happened once instead of each time a resource is changed.

Is the rendering jump from items being shown and then web component upgrade?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't be web component upgrade - that issue only really happens when the JS isn't loaded when the elements get rendered.

I think what is happening is when tree items are added as children to tree items, they're marked as nested and then the templating for the tree item runs again and that adds nested to the class. I'd think that would be fast enough we wouldn't notice the rendering, but maybe not.

Another thought is that this could be another issue similar to microsoft/fluentui-blazor#1082 - a similar situation is happening here where FluentTreeItem is setting the class attribute but the underlying tree item is setting it for itself as well (see above). Hrm...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I think I found something that works. If I add nested to the ones we know are nested, then it no longer jumps:

MetricsTreeJumpingNoMore

@foreach (var instrument in meterGroup.OrderBy(i => i.Name))
{
<FluentTreeItem Class="metrics-tree-item" Text="@instrument.Name" Data="@instrument" title="@instrument.Name" InitiallySelected="@(instrument.Name == _selectedInstrument?.Name && instrument.Parent.MeterName == _selectedMeter?.MeterName)" />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metrics-tree-item wasn't being used anymore.

@tlmii tlmii enabled auto-merge (squash) December 8, 2023 06:14
@tlmii tlmii merged commit 978d99b into dotnet:main Dec 8, 2023
8 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2024
@tlmii tlmii deleted the dev/workaround-tree-selection branch June 25, 2024 05:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants