Add details column to trace view, remove status badge and put counter badge on resource name #1788

adamint · 2024-01-23T20:12:05Z

Based on our conversations early this month, I added icons to running (success), finished, and exited resources. The unread error log badge was removed, instead using a counter badge over the resource name (with the same link to that resource's errored logs, but that can be removed if anyone feels it is unnecessary).

To standardize data grid displays while we work on a hierarchical data grid component, I added a details column to trace view so that all data grids contain one:

After we replace data grid components, all detail columns will be removed.

#1081 (additional issue is #1782), fixes #1534, fixes #1627

Microsoft Reviewers: Open in CodeFlow

JamesNK · 2024-01-23T22:41:38Z

To standardize data grid displays while we work on a hierarchical data grid component, I added a details column to trace view so that all data grids contain one:

We want to make rows consistently selectable. No one is confused about this page so moving away from what we want isn't necessary.

adamint · 2024-01-23T22:44:08Z

moving away from what we want isn't necessary.

unfortunately it is necessary. span details is currently completely inaccessible to keyboard and screen reader users with no workaround

@JamesNK

JamesNK · 2024-01-23T23:23:22Z

Ok if it is temporarily required.

Row selection really needs to be done. It keeps coming up. Investing resources in working around it is wasteful.

JamesNK

Overall looks good.

I noticed resource name lost its title at some point. Some code suggestions to add it back.

src/Aspire.Dashboard/Components/ResourcesGridColumns/ResourceNameDisplay.razor

tlmii · 2024-01-23T23:57:19Z

span details is currently completely inaccessible to keyboard and screen reader users with no workaround

Can we get specific about what doesn't work with the current grid as far as accessibility goes? I'm sure I'm missing something from the list, but here's a starting point:

Per-cell keyboard focus and navigation work on the current grid. You can use the arrow keys to move around the grid and give focus to various cells. There's no row selection, but being able to select each cell covers that scenario for now at least.
Selection does not work right now.
Presumably we need some indication to screen readers that a particular cell/row can be "executed".

For 2 The W3C examples (e.g. https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/data-grids/) use a keydown handler on cells. We could do the same.

For 3 it seems like it might be sufficient to give the div for the cell an aria-role of button. The W3C examples above use a hidden button, though off hand I don't see a reason why that'd be better.

Edit: I realized after hitting submit that this PR is probably not the right place for this discussion - I was just replying to that portion of the comments. I copied the comment over to #1081 (comment) as that seems like a little better place for it.

davidfowl · 2024-01-25T05:08:53Z

This looks pretty great! Good work @adamint !

drewnoakes

Very nice.

drewnoakes · 2024-01-25T05:21:14Z

src/Aspire.Dashboard/Components/ResourcesGridColumns/ResourceNameDisplay.razor

+@if (unviewedErrorCount > 0)
+{
+    <FluentCounterBadge Class="error-counter-badge" Max="9" BackgroundColor="@Color.Error" Color="Color.Fill"  Appearance="Appearance.Lightweight" Count="@unviewedErrorCount" HorizontalPosition="100">
+        <a href="@GetResourceErrorStructuredLogsUrl(Resource)" title="@FormatLogLinkTitle(unviewedErrorCount)"><FluentHighlighter HighlightedText="@FilterText" Text="@FormatName(Resource)" /></a>


I can imagine that having the resource's name link to its logs would be good for all resources, not just those with errors.

@drewnoakes do you think links appearing for all resources would be noisy?

It would probably be noisey. It's something to think about in the future. The PR is good as it is.

Do we still need to link to the logs from the resource name now that we're reinstated the "view logs" button?

# Conflicts: # src/Aspire.Dashboard/Components/Pages/Resources.razor

* Change default port for dashboard applicationUrl (#1853) Windows allows systems to exclude specific port ranges, after which attempts to bind them will fail. The ranges will vary from machine to machine, and can be viewed with the command `netsh interface ipv4 show excludedportrange protocol=tcp`. We're seeing users hitting issues with port 63824. While the excluded ranges will vary from machine to machine, it seems like these higher numbers are more likely to be excluded. This change moves the ports lower, which should help alleviate this issue. * Add details column to trace view, remove status badge and put counter badge on resource name (#1788) * add details column to trace detail * Update xlfs, style button * remove error badge * Put unread error counter on name * update trace detail details column to absolute width * Remove unused resx strings, re-add titles for resource name * Make error counter badge text white in dark theme * fixed variable name --------- Co-authored-by: Adam Ratzman <[email protected]> * Fix horizontal overflow in metrics popup (#1838) * Remove unused "known property" getter methods (#1856) This code is no longer used, so remove it. * Update dashboard README (#1854) * Fix trace detail page's gap between trace ticks (#1861) * Fix SQL server resource version (#1870) * Add Pomelo.EntityFrameworkCore.MySql component (#1161) * Add Pomelo.EntityFrameworkCore.MySql component. * Add Pomelo.EntityFrameworkCore.MySql tests. * Update XML documentation. * Update XML documentation to current conventions. * Add documentation for Pomelo metrics. * Use configuration schema generator. * Allow ServerVersion setting to be optional. * Remove (now optional) ServerVersion setting from test. * Remove MySqlConnector logging categories from Pomelo section. * Add Microsoft.Extensions.Configuration.Binder. * Add basic MySQL Aspire.Hosting tests. These were modeled on AddPostgresTests and AddRedisTests. * Add Pomelo EFCore integration tests. * Incorporate project change from dff1467. * PR feedback - Add log categories to ConfigurationSchema - Check the "API" boxes for the MySql components in the Progress doc - Issue a query in the functional test. Follow the same pattern as Oracle - Set the ServerVersion explicitly in the functional test, so the connection can be retried when the MySql server isn't running. --------- Co-authored-by: Eric Erhardt <[email protected]> * Fix TryAddWillNotAddTheSameLifecycleHook test. (#1862) * Use the right health check properties in ServiceBus README. (#1876) Fix #1715 * Trace detail page - customize peer icons based on span attributes (#1865) * Update SqlServerBuilderExtensions.cs (#1883) Follow up to #1870 * Fix Storage Queue and Cosmos EF READMEs (#1884) * Fix Storage Queue README The ConnectionStrings setting had the wrong name, so it doesn't work with what is in the code section above it. Fix #1679 * Fix CosmosDB README The extension method takes 2 strings: connection name and database name. Fix #873 * remove link in name column (#1886) Co-authored-by: Adam Ratzman <[email protected]> * Orleans: enable distributed tracing by default (#1858) * add failedtostart error icon to state (#1887) * add failedtostart error icon to state * add starting icon --------- Co-authored-by: Adam Ratzman <[email protected]> * Fix grid layout issue (#1889) * Update dependencies from https://github.com/microsoft/usvc-apiserver build 0.1.49+5 (#1890) Microsoft.DeveloperControlPlane.darwin-amd64 , Microsoft.DeveloperControlPlane.darwin-arm64 , Microsoft.DeveloperControlPlane.linux-amd64 , Microsoft.DeveloperControlPlane.linux-arm64 , Microsoft.DeveloperControlPlane.windows-386 , Microsoft.DeveloperControlPlane.windows-amd64 , Microsoft.DeveloperControlPlane.windows-arm64 From Version 0.1.48 -> To Version 0.1.49 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix inconsistent trace detail spacing (#1863) * Fix inconsistent trace detail spacing * Clean up * Playground app for SQL Server resources (uses EF). (#1895) * End to end SQL playground. * Branding changes for preview 4 (#1892) * Fix dashboard not working with two applicationUrls specified. (#1901) * Fix dashboard not working with two applicationUrls specified. * PR feedback. Defensive code. * Remove unnecessary code from AppHost in Orleans playground (#1904) * remove unnecessary code from Orleans AppHost * format comment in Orleans AppHost * Added a mongo playground project (#1921) * Consistent resource type summaries (#1772) * Removed static web assets from aspire hosting (#1923) - We're no longer embedding the dashboard so this shouldn't be required. * Transform outgoing address to try to find a resource match (#1932) * Improve environment variable debugging in the app model (#1746) * Move shared code to shared directory (#1751) * Fixed build from shared code change (#1937) * Allow collapsing operations in the trace view (#1323) Co-authored-by: James Newton-King <[email protected]> * Update Extensions to preview 1 versions (#1891) * Update dependencies from https://github.com/dotnet/arcade build 20240125.5 (#1947) Microsoft.DotNet.Arcade.Sdk , Microsoft.DotNet.Build.Tasks.Installers , Microsoft.DotNet.Build.Tasks.Workloads , Microsoft.DotNet.Helix.Sdk , Microsoft.DotNet.RemoteExecutor , Microsoft.DotNet.SharedFramework.Sdk , Microsoft.DotNet.XUnitExtensions From Version 8.0.0-beta.24060.4 -> To Version 8.0.0-beta.24075.5 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Update dependencies from https://github.com/dotnet/extensions build 20240126.7 (#1954) Microsoft.Extensions.Http.Resilience From Version 9.0.0-alpha.1.24076.5 -> To Version 9.0.0-preview.1.24076.7 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Fix dotnet watch. (#1933) * Fix dotnet watch. * Fix precondition in DcpHostService (#1943) * Fix precondition in DcpHostService Two lines are changed here, though it's the second (in `StopAsync`) that has a functional change. The first guard clause allows a null publisher, or a "dcp" publisher. The second clause would allow any non-null publisher, which looks like a bug to me. Specifically, the first half of the || excludes "dcp", then the second half allows it back in. This means that there's no point checking for "dcp" in the code as written. My interpretation of this code is that both clauses are intended to be the same, and this commit brings them in line with one another. * Deduplicate condition * Fix histogram chart calculation (#1968) Co-authored-by: Drew Noakes <[email protected]> * Fix exception when app host has no ASPNETCORE_URLS (#1970) If an app host project's launch profile doesn't specify a `launchUrl` or `applicationUrl` then the running app host won't have a `ASPNETCORE_URLS` environment variable. We want to pass this variable's value to the dashboard process, and currently if it's missing we throw. The dashboard executable has a fallback value when the `ASPNETCORE_URLS` variable is not present (of `http://localhost:18888`), so it's perfectly happy to run without this variable. This change causes the app host to fall back to that same default value for the dashboard's `ASPNETCORE_URLS` variable. While we could pass no value at all, and have the dashboard choose the default, it's more helpful to the user if we log the dashboard URL. This allows the user to find the dashboard, as the IDE will not be launching it automatically when in this state. * Revert state column badge changes (#1955) * revert state column badge changes * Slightly darken error in dark theme * Remove spurious VS-added itemgroups * Auto-close empty element --------- Co-authored-by: Adam Ratzman <[email protected]> * Update OpenTelemetry packages and use built-in metric methods (#1948) * Update OpenTelemetry packages - Update all the OpenTelemetry packages to their latest versions. - Use built-in methods to add .NET metrics. - Add suppression for new AoT warning on `AddSqlClientInstrumentation()`. * Resolve review feedback Mark as not AoT compatible due to dependency on OpenTelemetry.Instrumentation.SqlClient, which itself is not AoT compatible. * Prevent ANE when resource state is null (#1975) Ideally we'd make the field nullable on the proto definition, but that's a breaking change at this point. So instead, we pass an empty string. Other options are discussed in #1930. * Remove PublishBuildArtifacts (#1990) This already happens as part of the publish step in the build. Doing this over again (especially to the same container) will overwrite the previous results in some cases, and also unnecessarily duplicate files. It also will cause the wixpacks to get picked up for signing validation. * Updating some new projects to target net9.0 * Skip Pomelo tests failing due to Pomelo depending on EF 8.0 methods that were removed in 9.0 --------- Co-authored-by: Drew Noakes <[email protected]> Co-authored-by: Adam Ratzman <[email protected]> Co-authored-by: Adam Ratzman <[email protected]> Co-authored-by: James Newton-King <[email protected]> Co-authored-by: Mitch Denny <[email protected]> Co-authored-by: Bradley Grainger <[email protected]> Co-authored-by: Eric Erhardt <[email protected]> Co-authored-by: David Fowler <[email protected]> Co-authored-by: Reuben Bond <[email protected]> Co-authored-by: Tim Mulholland <[email protected]> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Jose Perez Rodriguez <[email protected]> Co-authored-by: Vincent Hoogendoorn <[email protected]> Co-authored-by: Stephan Bauer <[email protected]> Co-authored-by: Martin Costello <[email protected]> Co-authored-by: Matt Mitchell <[email protected]>

Adam Ratzman added 5 commits January 23, 2024 11:32

add details column to trace detail

2956d12

Update xlfs, style button

5cb939a

remove error badge

465b920

Put unread error counter on name

a729637

update trace detail details column to absolute width

adf0537

adamint requested review from DamianEdwards, JamesNK, drewnoakes, leslierichardson95, tlmii and kvenkatrajan January 23, 2024 20:12

dotnet-issue-labeler bot added the area-dashboard label Jan 23, 2024

adamint requested a review from davidfowl January 23, 2024 20:41

JamesNK reviewed Jan 23, 2024

View reviewed changes

src/Aspire.Dashboard/Components/ResourcesGridColumns/ResourceNameDisplay.razor Outdated Show resolved Hide resolved

src/Aspire.Dashboard/Components/ResourcesGridColumns/ResourceNameDisplay.razor Outdated Show resolved Hide resolved

tlmii mentioned this pull request Jan 23, 2024

Standardize grid selection and selected row display #1081

Closed

Adam Ratzman added 2 commits January 24, 2024 11:12

Remove unused resx strings, re-add titles for resource name

97fefb8

Make error counter badge text white in dark theme

8d98532

adamint requested a review from JamesNK January 24, 2024 16:39

fixed variable name

a22a8dc

JamesNK approved these changes Jan 24, 2024

View reviewed changes

drewnoakes approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into dev/adamint/1081-standardize-grid-selection

e4ff870

# Conflicts: # src/Aspire.Dashboard/Components/Pages/Resources.razor

adamint enabled auto-merge (squash) January 25, 2024 20:21

adamint merged commit 838d0a1 into dotnet:main Jan 25, 2024
8 checks passed

davidfowl mentioned this pull request Jan 27, 2024

use icons for service status in dashboard #484

Closed

github-actions bot locked and limited conversation to collaborators Apr 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add details column to trace view, remove status badge and put counter badge on resource name #1788

Add details column to trace view, remove status badge and put counter badge on resource name #1788

adamint commented Jan 23, 2024 •

edited

Loading

JamesNK commented Jan 23, 2024

adamint commented Jan 23, 2024 •

edited

Loading

JamesNK commented Jan 23, 2024

JamesNK left a comment •

edited

Loading

tlmii commented Jan 23, 2024 •

edited

Loading

davidfowl commented Jan 25, 2024

drewnoakes left a comment

drewnoakes Jan 25, 2024

adamint Jan 25, 2024

JamesNK Jan 25, 2024

drewnoakes Jan 25, 2024

Add details column to trace view, remove status badge and put counter badge on resource name #1788

Add details column to trace view, remove status badge and put counter badge on resource name #1788

Conversation

adamint commented Jan 23, 2024 • edited Loading

Microsoft Reviewers: Open in CodeFlow

JamesNK commented Jan 23, 2024

adamint commented Jan 23, 2024 • edited Loading

JamesNK commented Jan 23, 2024

JamesNK left a comment • edited Loading

Choose a reason for hiding this comment

tlmii commented Jan 23, 2024 • edited Loading

davidfowl commented Jan 25, 2024

drewnoakes left a comment

Choose a reason for hiding this comment

drewnoakes Jan 25, 2024

Choose a reason for hiding this comment

adamint Jan 25, 2024

Choose a reason for hiding this comment

JamesNK Jan 25, 2024

Choose a reason for hiding this comment

drewnoakes Jan 25, 2024

Choose a reason for hiding this comment

adamint commented Jan 23, 2024 •

edited

Loading

adamint commented Jan 23, 2024 •

edited

Loading

JamesNK left a comment •

edited

Loading

tlmii commented Jan 23, 2024 •

edited

Loading