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.

Sign up for GitHub

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

Jump to bottom

Generate resource type filters from data #1221

Merged
merged 4 commits into from
Dec 7, 2023
Merged

Generate resource type filters from data #1221

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9066ce6
Generate resource type filters from data
drewnoakes Dec 6, 2023
97770c5
Reinstate indeterminate state
drewnoakes Dec 6, 2023
111a572
Merge remote-tracking branch 'upstream/main' into dynamic-resource-types
drewnoakes Dec 6, 2023
f309b30
Change ShowIndeterminate value
drewnoakes Dec 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions src/Aspire.Dashboard/Components/Pages/Resources.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,28 @@
<FluentToolbar Orientation="Orientation.Horizontal">
<h1 slot="label">Resources</h1>
<FluentButton id="typeFilterButton" slot="end"
Appearance="@(AreAllTypesVisible ? Appearance.Stealth : Appearance.Accent)"
Appearance="@(AreAllTypesVisible is true ? Appearance.Stealth : Appearance.Accent)"
IconEnd="@(new Icons.Regular.Size24.Filter())"
@onclick="() => _isTypeFilterVisible = !_isTypeFilterVisible"
Title="@(AreAllTypesVisible ? "Type Filter: All Types Visible" : "Type Filter: Filtered")"
aria-label="@(AreAllTypesVisible ? "Type Filter: All Types Visible" : "Type Filter: Filtered")" />
Title="@(AreAllTypesVisible is true ? "Type Filter: All Types Visible" : "Type Filter: Filtered")"
aria-label="@(AreAllTypesVisible is true ? "Type Filter: All Types Visible" : "Type Filter: Filtered")" />
<FluentPopover AnchorId="typeFilterButton" @bind-Open="_isTypeFilterVisible">
<Header>Resource Types</Header>
<Body>
<FluentStack Orientation="Orientation.Vertical">
<FluentCheckbox Label="All" CheckStateChanged="HandleTypeFilterShowAllChanged"
Copy link
Member

Choose a reason for hiding this comment

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

The current code doesn't seem to respond to clicking the All checkbox as expected, but I think its because of the ShowIndeterminate parameter in my other comment and not because of the changes from CheckState/CheckStateChanged to @bind-CheckState. I think when it is checked and you click it, it goes to indeterminate state, but then immediately gets flipped back to true because nothing changed with the other checkboxes.

ThreeState="true" ShowIndeterminate="false"
CheckState="_allTypesVisible" />
<FluentCheckbox @bind-Value="_areProjectsVisible" Label="Projects" @bind-Value:after="HandleTypeFilterTypeChanged" />
<FluentCheckbox @bind-Value="_areContainersVisible" Label="Containers" @bind-Value:after="HandleTypeFilterTypeChanged" />
<FluentCheckbox @bind-Value="_areExecutablesVisible" Label="Executables" @bind-Value:after="HandleTypeFilterTypeChanged" />
<FluentCheckbox
Copy link
Member

Choose a reason for hiding this comment

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

Looks like you got rid of the indeterminate (i.e. "some are checked some are not") state? Was that an intentional change?

Copy link
Member Author

Choose a reason for hiding this comment

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

I had the impression it wasn't actually showing intederminate state in the previous code because of ShowIndeterminate="false", but maybe that's just clearing indeterminate state if present.

I will reinstate the indeterminate state.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, the naming is a little confusing. ShowIndeterminate actually controls whether the user can toggle between on/off and indeterminate with mouse clicks. We don't want that, so it's false. ThreeState parameter controls whether the component will even display the three states in any situation (including programmatic, which we use), so that one was true.

You can see some docs and examples here if it helps: https://www.fluentui-blazor.net/Checkbox#fluentcheckbox-class_properties

Copy link
Member

Choose a reason for hiding this comment

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

To clarify the existing behavior:

The "All" checkbox would automatically toggle to checked (if all other checkboxes were checked), unchecked (if no other checkboxes were checked) or indeterminate (any other combination of checks).
The user clicking it wouldn't ever toggle it to indeterminate, just between checked/unchecked - and those actions would impact all the other checkboxes as appropriate.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I think it'll be straightforward to restore this behaviour.

Copy link
Member Author

Choose a reason for hiding this comment

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

I checked the behaviour on main. Here are some recordings:

tri-state-before

When starting from no selections and adding, it appears indeterminate:

tri-state-before-2

Copy link
Member Author

Choose a reason for hiding this comment

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

I played around with this for a while and am can't tell why it wouldn't work. Sticking a breakpoint in the property getter the CheckState value is bound to shows it's getting a null but displaying as a false. Anyone got any thoughts or ideas here?

Label="All"
ThreeState="true"
ShowIndeterminate="false"
@bind-CheckState="AreAllTypesVisible" />
@foreach (string resourceType in _allResourceTypes) {
bool isChecked = _visibleResourceTypes.Contains(resourceType);
<FluentCheckbox
Label="@resourceType"
@bind-Value:get="isChecked"
@bind-Value:set="c => OnResourceTypeVisibilityChanged(resourceType, c)"
/>
}
</FluentStack>
</Body>
</FluentPopover>
Expand Down
55 changes: 34 additions & 21 deletions src/Aspire.Dashboard/Components/Pages/Resources.razor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Collections.Immutable;
using Aspire.Dashboard.Extensions;
using Aspire.Dashboard.Model;
using Aspire.Dashboard.Otlp.Model;
using Aspire.Dashboard.Otlp.Storage;
Expand All @@ -25,45 +27,56 @@ public partial class Resources : ComponentBase, IDisposable
private ResourceViewModel? SelectedResource { get; set; }

private bool Filter(ResourceViewModel resource)
=> ((resource.ResourceType == "Project" && _areProjectsVisible) ||
(resource.ResourceType == "Container" && _areContainersVisible) ||
(resource.ResourceType == "Executable" && _areExecutablesVisible)) &&
=> _visibleResourceTypes.Contains(resource.ResourceType) &&
(resource.Name.Contains(_filter, StringComparison.CurrentCultureIgnoreCase) ||
(resource is ContainerViewModel containerViewModel &&
containerViewModel.Image.Contains(_filter, StringComparison.CurrentCultureIgnoreCase)));

private readonly Dictionary<string, ResourceViewModel> _resourcesMap = new();
private readonly CancellationTokenSource _watchTaskCancellationTokenSource = new();
private readonly Dictionary<string, ResourceViewModel> _resourcesMap = [];
// TODO populate resource types from server data
private readonly ImmutableArray<string> _allResourceTypes = ["Project", "Executable", "Container"];
private readonly HashSet<string> _visibleResourceTypes;
private string _filter = "";
private bool _isTypeFilterVisible;
private bool? _allTypesVisible = true;
private bool _areProjectsVisible = true;
private bool _areContainersVisible = true;
private bool _areExecutablesVisible = true;

private bool AreAllTypesVisible => _areProjectsVisible && _areContainersVisible && _areExecutablesVisible;
public Resources()
{
_visibleResourceTypes = new HashSet<string>(_allResourceTypes, StringComparers.ResourceType);
}

private void HandleTypeFilterShowAllChanged(bool? newValue)
protected void OnResourceTypeVisibilityChanged(string resourceType, bool isVisible)
{
if (newValue.HasValue)
if (isVisible)
{
_visibleResourceTypes.Add(resourceType);
}
else
{
_allTypesVisible = _areProjectsVisible = _areContainersVisible = _areExecutablesVisible = newValue.Value;
_visibleResourceTypes.Remove(resourceType);
}
}

private void HandleTypeFilterTypeChanged()
private bool? AreAllTypesVisible
{
if (_areProjectsVisible && _areContainersVisible && _areExecutablesVisible)
{
_allTypesVisible = true;
}
else if (!_areProjectsVisible && !_areContainersVisible && !_areExecutablesVisible)
get
{
_allTypesVisible = false;
return _visibleResourceTypes.SetEquals(_allResourceTypes)
? true
: _visibleResourceTypes.Count == 0
? false
: null;
}
else
set
{
_allTypesVisible = null;
if (value is true)
{
_visibleResourceTypes.UnionWith(_allResourceTypes);
}
else if (value is false)
{
_visibleResourceTypes.Clear();
}
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/Aspire.Dashboard/Extensions/StringComparers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Aspire.Dashboard.Extensions;

internal static class StringComparers
{
public static StringComparer ResourceType => StringComparer.Ordinal;
}
Comment on lines +6 to +9
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a convention I've seen used in a few places, giving semantic names to different domains of strings, ensuring they're compared in a uniform way.