Move filters into a menu, add a remove all filters button #8123

adamint · 2025-03-17T20:42:51Z

Description

This PR removes the current horizontal filter view, moving the filters into a fluent menu with a counter badge to hint at the number of filters set.

Inside the menu, set filters are presented first, then a divider and a clear all filters item. Maybe we could think of icons to set for each of the filter types?

If no filters are present, "No filters" continues to be shown.

Fixes #5673

Checklist

Is this feature complete?
- Yes. Ready to ship.
- No. Follow-up changes expected.
Are you including unit tests for the changes and scenario tests if relevant?
- Yes
- No
Did you add public API?
- Yes
  - If yes, did you have an API Review for it?
    - Yes
    - No
  - Did you add <remarks /> and <code /> elements on your triple slash comments?
    - Yes
    - No
- No
Does the change make any security assumptions or guarantees?
- Yes
  - If yes, have you done a threat model and had a security review?
    - Yes
    - No
- No
Does the change require an update in our Aspire docs?
- Yes
  - Is this introducing a breaking change?
    - Yes
      - Link to aspire-docs issue (please use this breaking-change template):
    - No
      - Link to aspire-docs issue (please use this doc-idea template):
- No

Copilot

Pull Request Overview

This PR refactors the filter UI on the Structured Logs page by moving filters into a Fluent menu and adding a “remove all filters” option.

Introduces a new helper method (GetFilterMenuItems) to generate menu items for existing filters and the clear-all option.
Adds a new DisableIcon parameter to the AspireMenuButton to allow conditional rendering of the icon.
Updates the StructuredLogs view to wrap the new menu button in a counter badge indicating the number of active filters.

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.

File	Description
src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor.cs	Added GetFilterMenuItems method to generate filter menu items, including a clear-all option.
src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs	Introduced DisableIcon parameter to offer control over icon rendering.
src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor	Updated rendering logic to conditionally include the icon based on DisableIcon.
src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor	Replaced inline filtering buttons with a counter badge wrapping a menu button.

Files not reviewed (1)

src/Aspire.Dashboard/wwwroot/css/app.css: Language not supported

Comments suppressed due to low confidence (1)

src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor:80

Consider adding unit tests to cover the new filter menu functionality and its interactions, ensuring that the filtering and clear-all behavior work as expected.

<AspireMenuButton DisableIcon="true" Text="@FilterLoc[nameof(StructuredFiltering.Filters)]" Items="@GetFilterMenuItems()" />

adamint · 2025-03-17T20:43:28Z

src/Aspire.Dashboard/Components/Pages/StructuredLogs.razor

-                    <FluentButton slot="end" Appearance="Appearance.Outline" OnClick="() => OpenFilterAsync(filter)" class="telemetry-filter-button" title="@filter.GetDisplayText(FilterLoc)">@filter.GetDisplayText(FilterLoc)</FluentButton>
-                }
+                <div slot="end">
+                    <FluentCounterBadge HorizontalPosition="70" Count="@(ViewModel.Filters.Count)" Appearance="Appearance.Accent">


setting slot=end onto counter badge doesn't work, maybe a bug in the component? I can check

Yeah, this is a bug in that component

microsoft/fluentui-blazor#3542

mitchdenny · 2025-03-17T22:28:59Z

The placement of Filters and No filters seems strange to me. I think maybe its the fact that it changes from Filters to No filters that is putting me off. I wonder if when no filters are set it still says Filters, but doesn't have the badge.

I'm no UX expert though.

mitchdenny · 2025-03-17T22:29:45Z

Actually, the fact that a badge is being used is also a bit odd I think. Perhaps changing the colour of the button itself?

JamesNK · 2025-03-18T03:09:49Z

A long name or value overflows the menu item. A very long name here should be trimmed with ....

JamesNK · 2025-03-18T03:13:26Z

I'm guessing there's no limit on the height or the menu:

It might be a problem, but realistically there aren't going to be that many filters added so lets not worry right now.

JamesNK · 2025-03-18T03:15:34Z

Refreshing the page while there is a filter causes the badge number to move.

Add a new filter:

Refresh the page:

JamesNK · 2025-03-18T03:16:29Z

Traces page has the same filters. They're not updated.

src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs

JamesNK · 2025-03-18T03:20:58Z

Remove all just updates the filter UI. It doesn't remove the filters from the query string or update the page:

JamesNK · 2025-03-18T03:52:37Z

Actually, the fact that a badge is being used is also a bit odd I think. Perhaps changing the colour of the button itself?

I think it's ok. Lets you know how many filters there are.

adamint · 2025-03-19T15:37:13Z

Refreshing the page while there is a filter causes the badge number to move.

Add a new filter:

Refresh the page:

This is due to a fluentui-blazor bug that was fixed on 3/10 and will be included in the next release. The 'true' positioning is the former (what you see after adding a filter)

adamint · 2025-03-19T15:46:21Z

The placement of Filters and No filters seems strange to me. I think maybe its the fact that it changes from Filters to No filters that is putting me off. I wonder if when no filters are set it still says Filters, but doesn't have the badge.

I'm no UX expert though.

@JamesNK?

JamesNK · 2025-03-20T01:23:57Z

tests/Shared/AsyncTestHelpers.cs

            if (i > 0)
            {
-                await Task.Delay((i + 1) * (i + 1) * 10);
+                await Task.Delay((i + 1) * (i + 1) * 10 * 5);


What caused this to timeout?

JamesNK · 2025-03-20T01:54:56Z

The placement of Filters and No filters seems strange to me. I think maybe its the fact that it changes from Filters to No filters that is putting me off. I wonder if when no filters are set it still says Filters, but doesn't have the badge.
I'm no UX expert though.

@JamesNK?

I think it is Good Enough(tm) to merge. Nothing stops it changing later. People can use and give feedback.

Move filters into a menu, add a remove all filters button

1aa1536

adamint requested review from JamesNK and Copilot March 17, 2025 20:42

github-actions bot added the area-dashboard label Mar 17, 2025

Copilot AI reviewed Mar 17, 2025

View reviewed changes

adamint commented Mar 17, 2025

View reviewed changes

JamesNK reviewed Mar 18, 2025

View reviewed changes

src/Aspire.Dashboard/Components/Controls/AspireMenuButton.razor.cs Outdated Show resolved Hide resolved

add maximum width to menu item

c577079

rename DisableIcon->HideIcon

0a3ab63

adamint added 2 commits March 19, 2025 11:46

make sure to update url after clear filters

45c020d

Add filters menu to traces page

6055718

adamint requested a review from JamesNK March 19, 2025 15:53

adamint added 3 commits March 19, 2025 12:08

remove redundant attribute

99d4b95

add necessary service to test

cb3a3dd

Increase timeout for flaky test

834b140

adamint requested review from eerhardt and radical as code owners March 19, 2025 16:12

JamesNK reviewed Mar 20, 2025

View reviewed changes

JamesNK approved these changes Mar 20, 2025

View reviewed changes

JamesNK merged commit 358405c into dotnet:main Mar 20, 2025
166 checks passed

davidfowl mentioned this pull request Apr 7, 2025

What's new in .NET Aspire 9.2 dotnet/docs-aspire#2877

Merged

github-actions bot locked and limited conversation to collaborators Apr 19, 2025

Move filters into a menu, add a remove all filters button #8123

Move filters into a menu, add a remove all filters button #8123

Uh oh!

Conversation

adamint commented Mar 17, 2025

Description

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

adamint Mar 17, 2025

Choose a reason for hiding this comment

Uh oh!

adamint Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

adamint Mar 19, 2025

Choose a reason for hiding this comment

Uh oh!

mitchdenny commented Mar 17, 2025

Uh oh!

mitchdenny commented Mar 17, 2025

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

JamesNK commented Mar 18, 2025

Uh oh!

adamint commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adamint commented Mar 19, 2025

Uh oh!

JamesNK Mar 20, 2025

Choose a reason for hiding this comment

Uh oh!

JamesNK commented Mar 20, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adamint commented Mar 19, 2025 •

edited

Loading