-
Notifications
You must be signed in to change notification settings - Fork 597
Remember resource filter states #8593
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
base: main
Are you sure you want to change the base?
Remember resource filter states #8593
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/Aspire.Dashboard/Utils/DashboardUrls.cs:19
- The URL base no longer starts with a leading slash as it used to ($"/{ResourcesBasePath}"). This may cause issues with URL resolution; consider reintroducing the '/' prefix if required for proper routing.
var url = ResourcesBasePath;
var preselectedVisibleResourceTypes = VisibleTypes?.Split(',').ToHashSet(); | ||
var preselectedVisibleResourceStates = VisibleStates?.Split(',').ToHashSet(); | ||
var preselectedVisibleResourceHealthStates = VisibleHealthStates?.Split(',').ToHashSet(); | ||
var preselectedHiddenResourceTypes = HiddenTypes?.Split(' ').Select(StringUtils.Unescape).ToHashSet(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if a value (type, state, health state) contains a space?
Why did you change from comma to space?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens if a value (type, state, health state) contains a space?
The space is serialized to a +. Then, when appending to the URL, QueryHelpers.AddQueryString
will escape the + to %2B. Reverse process happens when reading from the query param.
Why did you change from comma to space?
I noticed that this was the only instance where a space was not being used to separate separate filters. In telemetry filters, space is used in the URL.
Resource filters are in the querystring when returning to the resources pages, but they aren't used: Test all the scenarios where you could add/remove filters, and navigate to and from the resources page, e..g between table and graph, graph and another page, another page back to graph, table to another page, etc, etc |
Seems this is tricky to get right. Are component tests needed here to avoid regressions? |
I added one to simulate the tricky case (navigation to base path, then filters are added to the url, then parameters update and filters should be applied). Do you think it's sufficient? |
Description
Remembers the disabled states in this page. Moves string escape/unescape functions to StringUtils.
Fixes #3647
Checklist
<remarks />
and<code />
elements on your triple slash comments?breaking-change
template):doc-idea
template):