Create hover tokens for grid row hover effect #4892

tlmii · 2024-07-15T06:45:41Z

Finally got around to taking a stab at #2281 to create hover tokens for the neutral-layer-1/neutral-layer-2 colors we use as our primary background colors from the current styles provided by the designers. Those tokens are not automatically provided by Fluent so we need to create them ourselves.

Creates hover tokens for neutral-layer-1/neutral-layer-2 that we use as primary background colors for the current styles provided by the designers. Those tokens aren't automatically provided by Fluent like other -hover variants so we need to create them ourselves
The above tokens are created as generically as possible so that we can tweak them as we want. E.g. if we decide that the layer 2 light color is too light/dark, we can change the neutralLayer2HoverLightDelta up/down by 1 to move it to the next value in the color palette.
Ties the new tokens into the FluentDataGrid's ShowHover functionality.
Tweaks some of the hover effects on buttons in the grids to make them make a little more sense when the row is hovered

I gave a look at all of the pages as I went through this, but I'm sure I missed some things that don't look quite right with this change. Will try to give it another pass tomorrow, but wanted to get the PR out for others eyes as well.

Microsoft Reviewers: Open in CodeFlow

JamesNK

I played around with it briefly and it seems to work in the places I tested.

Visually it looks good. The main thing I don't like is the cursor has changed to a pointer. I think it's confusing to have that cursor when there is nothing to click.

The simple solution is to revert the cursor change, but I'm still interested in making rows clickable. That would be a nice improvement. But probably not in the scope of this PR.

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

src/Aspire.Dashboard/wwwroot/js/app-theme.js

tlmii · 2024-07-16T07:18:43Z

The main thing I don't like is the cursor has changed to a pointer. I think it's confusing to have that cursor when there is nothing to click.

The simple solution is to revert the cursor change, but I'm still interested in making rows clickable. That would be a nice improvement. But probably not in the scope of this PR.

@vnbaaij Have you gotten any other feedback about this? It does seem like the cursor change should be tied to the row being selectable, not just to the hover effect.

@JamesNK For now I overrode it with a more specific selector to put it back to auto.

vnbaaij · 2024-07-16T07:32:49Z

I haven't seen any remarks on that. But think it makes sense to only have a special cursor when needed.

@JamesNK we have the OnRowClick event since a couple of versions ago

JamesNK · 2024-07-16T08:04:21Z

I haven't seen any remarks on that. But think it makes sense to only have a special cursor when needed.

+1

@JamesNK we have the OnRowClick event since a couple of versions ago

Great! We should update Aspire to use it.

JamesNK · 2024-07-16T10:12:04Z

@tlmii I tested it out and it looks good except the change to remove the pointer cursor also removed it from the trace detail page. Rows are already clickable on trace detail so we want to keep it there.

Alternatively, allow for the pointer cursor and use OnRowClick to make rows clickable. That might require a FluentUI update. Or it might be a large amount of work. Could be done in a future PR.

tlmii · 2024-07-16T16:56:52Z

@tlmii I tested it out and it looks good except the change to remove the pointer cursor also removed it from the trace detail page. Rows are already clickable on trace detail so we want to keep it there.

Alternatively, allow for the pointer cursor and use OnRowClick to make rows clickable. That might require a FluentUI update. Or it might be a large amount of work. Could be done in a future PR.

Whoops. I updated the selector on trace detail to have higher specificity. Definitely agree on making everything clickable in the future and also that that should be in a future PR. Trying to keep this one minimal.

tlmii · 2024-07-16T18:33:11Z

/azp run

azure-pipelines · 2024-07-16T18:33:23Z

Azure Pipelines successfully started running 1 pipeline(s).

JamesNK · 2024-07-16T22:49:56Z

@tlmii I think the template tests need to be updated. They examine HTML so it's likely hover is changing the HTML output.

JamesNK · 2024-07-17T05:41:33Z

tests/Aspire.Workload.Tests/WorkloadTestsBase.cs

@@ -61,7 +61,7 @@ protected static async Task<ResourceRow[]> CheckDashboardHasResourcesAsync(IPage
            await Task.Delay(500);

            // _testOutput.WriteLine($"Checking for rows again");
-            ILocator rowsLocator = dashboardPage.Locator("//fluent-data-grid-row[@class='resource-row']");
+            ILocator rowsLocator = dashboardPage.Locator("//fluent-data-grid-row[@class='hover resource-row']");


Hmm, I don't like that this requires the exact right classes be specified in the query. It's very brittle. I wonder if there is something like the browser's getElementsByClassName?

Unrelated to this change though.

Yeah it seems a little brittle. Maybe just switching on the row type instead might be better (to skip header rows)? Even that seems a little brittle. But I didn't even realize this existed until you pointed it out, so I wasn't trying to delve into it too far yet, just trying to unblock.

I was hit by it last week. I vented here: #4834

I'll add class name brittleness to the list

commit d5231fd Author: Ankit Jain <radical@gmail.com> Date: Wed Jul 17 15:58:21 2024 -0400 Combine NonComponentReferenceForTests, and ComponentReferenceForTests into AspireProjectOrPackageReference commit d805dc9 Author: Ankit Jain <radical@gmail.com> Date: Wed Jul 17 15:41:08 2024 -0400 Rename Aspire.Testing.Repo to Aspire.RepoTesting - feedback from @ eerhardt commit 45524c3 Merge: 2b3cce9 cd6f024 Author: Ankit Jain <radical@gmail.com> Date: Wed Jul 17 15:38:48 2024 -0400 Merge remote-tracking branch 'origin/main' into tests-out-of-repo-targets commit cd6f024 Author: Mitch Denny <midenn@microsoft.com> Date: Thu Jul 18 03:21:17 2024 +1000 Disable local auth for service bus. (dotnet#4938) commit 752bc58 Author: Mitch Denny <midenn@microsoft.com> Date: Thu Jul 18 03:21:05 2024 +1000 DisableLocalAuth for AppConfig. (dotnet#4774) * DisableLocalAuth for AppConfig. * Update playground and test. commit 54d47fa Author: MerlinBot <MerlinBot> Date: Tue Jul 16 21:21:17 2024 +0000 This pull request includes baselines **with an expiration date of 180 days from now** automatically generated for your 1ES PT-based pipelines. Complete this pull request as soon as possible to make sure that your pipeline becomes compliant. Longer delays in completing this PR can trigger additional emails or S360 alerts in the future. 1ES PT Auto-baselining feature helps capture existing violations in your repo and ensures to break your pipeline only for newly introduced SDL violations after baselining. Running SDL tools in break mode is required for your pipeline to be compliant. Go to https://aka.ms/1espt-autobaselining for more details. **Please do not Abandon this PR.** Please reach out to 1ES PT for support. More details: https://eng.ms/docs/cloud-ai-platform/devdiv/one-engineering-system-1es/1es-docs/1es-pipeline-templates/support commit 12ad5c3 Author: Tim Mulholland <tlmii@users.noreply.github.com> Date: Wed Jul 17 06:31:12 2024 -0700 Create hover tokens for grid row hover effect (dotnet#4892) commit 85802db Author: James Newton-King <james@newtonking.com> Date: Wed Jul 17 11:43:37 2024 +0800 Revert change to always display first endpoint (dotnet#4935) commit bab8610 Author: James Newton-King <james@newtonking.com> Date: Wed Jul 17 11:43:23 2024 +0800 Add producer-consumer telemetry example to stress app (dotnet#4927) commit 6ba7b1e Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Date: Wed Jul 17 03:05:21 2024 +0000 Bump the orleans group with 5 updates (dotnet#4914) * Bump the orleans group with 5 updates Bumps the orleans group with 5 updates: | Package | From | To | | --- | --- | --- | | Microsoft.Orleans.Client | `8.1.0` | `8.2.0` | | Microsoft.Orleans.Sdk | `8.1.0` | `8.2.0` | | Microsoft.Orleans.Clustering.AzureStorage | `8.1.0` | `8.2.0` | | Microsoft.Orleans.Persistence.AzureStorage | `8.1.0` | `8.2.0` | | Microsoft.Orleans.Server | `8.1.0` | `8.2.0` | Updates `Microsoft.Orleans.Client` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Sdk` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Clustering.AzureStorage` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Sdk` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Persistence.AzureStorage` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Server` from 8.1.0 to 8.2.0 Updates `Microsoft.Orleans.Sdk` from 8.1.0 to 8.2.0 --- updated-dependencies: - dependency-name: Microsoft.Orleans.Client dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Sdk dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Clustering.AzureStorage dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Sdk dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Persistence.AzureStorage dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Server dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans - dependency-name: Microsoft.Orleans.Sdk dependency-type: direct:production update-type: version-update:semver-minor dependency-group: orleans ... Signed-off-by: dependabot[bot] <support@github.com> * error --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Dan Moseley <danmose@microsoft.com> commit 3a0e547 Author: James Newton-King <james@newtonking.com> Date: Wed Jul 17 09:40:13 2024 +0800 Fix metrics page (dotnet#4934) commit 2b3cce9 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 21:19:27 2024 -0400 comment commit 5fe9ab1 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 21:14:31 2024 -0400 Add comment, and simple cleanup commit 610bf1f Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 21:05:15 2024 -0400 Add README.md for Shared/RepoTesting commit 54b2037 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 21:01:38 2024 -0400 fix merge commit a15b208 Merge: 994695a 1f94dac Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 21:00:13 2024 -0400 Merge remote-tracking branch 'origin/main' into tests-out-of-repo-targets # Conflicts: # tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj commit 1f94dac Author: James Newton-King <james@newtonking.com> Date: Wed Jul 17 08:42:10 2024 +0800 Add span links to dashboard UI (dotnet#4805) commit cc32ebd Author: Eric Erhardt <eric.erhardt@microsoft.com> Date: Tue Jul 16 19:28:21 2024 -0500 Extract Aspire.Hosting.Python.Tests project (dotnet#4915) Contributes to dotnet#4294 commit 994695a Merge: 065dc6e 6de43ed Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 19:16:21 2024 -0400 Merge remote-tracking branch 'origin/main' into tests-out-of-repo-targets # Conflicts: # tests/testproject/TestProject.AppHost/TestProject.AppHost.csproj commit 065dc6e Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 19:14:22 2024 -0400 address review feedback from @ eerhardt commit 1326f38 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 19:10:54 2024 -0400 address review feedback from @ eerhardt commit b41f773 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 18:53:28 2024 -0400 Update tests/Aspire.Hosting.Tests/Aspire.Hosting.Tests.csproj Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com> commit 1fe6b17 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:38:38 2024 -0400 Move Directory.Packages.Helix.props to RepoTesting .. as it is only used by that. commit c532b48 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:36:03 2024 -0400 update docs commit b591ceb Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:23:45 2024 -0400 update docs commit 6a7997f Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:06:47 2024 -0400 Fix up EndToEnd tests - `TestsRunningOutsideOfRepo` now means that the whole project builds from source outside the repo. - But for `EndToEnd` tests only supporting test project is built in that mode. - So, update the property and the corresponding `#define` to reflect that - And track changes from adding the new props/targets commit 5ca6430 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:18:10 2024 -0400 Track changes in Aspire.Hosting.Tests commit 2a92aeb Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:17:50 2024 -0400 Track changes in Workload tests commit e577386 Author: Ankit Jain <radical@gmail.com> Date: Tue Jul 16 02:16:34 2024 -0400 Remove the redundant Directory.Build.props for various projects .. which were there essentially to import Hosting.props/targets . Instead, now the `Aspire.Testing.Repo.{props,targets}` are imported by default, which decide how to get the hosting targets. commit 50235a1 Author: Ankit Jain <radical@gmail.com> Date: Fri Jul 12 21:34:56 2024 -0400 Extract bits required for running tests outside of repo into .. `Aspire.Testing.Repo.{props,targets}`. From the comments: ``` This provides support for running tests outside of the repo, for example on a helix agent. - For this you need the source of the tests, and any dependencies. - Instead of direct `ProjectReferences` to the various Aspire hosting, and component projects use `@(ComponentReferenceForTests)`, and @(NonComponentReferenceForTests)`. - These are converted to `ProjectReference` when `$(TestsRunningOutsideOfRepo) != true`. - But converted to `PackageReference` when `$(TestsRunningOutsideOfRepo) == true`. - To allow building such test projects, the build is isolated and patched to build outside the repo by adding appropriate `Directory.Build.{props,targets}`, and `Directory.Packages.props` - and using a custom `nuget.config` which resolves the Aspire packages from the locally built packages - and a `Directory.Packages.Versions.props` is generated with PackageVersions taken from the repo - This also adds properties named in `@(PropertyForHelixRun)` from the repo, like `$(NetCurrent)` ``` commit edf4b75 Author: Ankit Jain <radical@gmail.com> Date: Fri Jul 12 21:30:12 2024 -0400 move out more shared files from workload testing for use in other projects commit 6e07d55 Author: Ankit Jain <radical@gmail.com> Date: Fri Jul 12 21:26:08 2024 -0400 Add new target to generate a Packages.Versions.props with all the relevant properties, and remove old custom tasks commit 9e519b3 Author: Ankit Jain <radical@gmail.com> Date: Fri Jul 12 21:24:29 2024 -0400 Move '_ExtractTestClassNames' target to tests/Directory.Build.targets for more common use # Conflicts: # playground/Directory.Packages.props # tests/Directory.Build.props # tests/Directory.Build.targets # tests/Shared/RepoTesting/Directory.Build.props # tests/Shared/RepoTesting/Directory.Build.targets # tests/Shared/RepoTesting/README.md # tests/testproject/TestProject.AppHost/TestProject.AppHost.csproj

tlmii requested review from JamesNK, drewnoakes, leslierichardson95, adamint and kvenkatrajan July 15, 2024 06:45

dotnet-issue-labeler bot added the area-dashboard label Jul 15, 2024

build-analysis bot mentioned this pull request Jul 15, 2024

Failing test Aspire.Hosting.Redis.Tests.RedisFunctionalTests.WithDataVolumeShouldPersistStateBetweenUsages #4863

Closed

JamesNK reviewed Jul 15, 2024

View reviewed changes

src/Aspire.Dashboard/Components/Pages/TraceDetail.razor Outdated Show resolved Hide resolved

src/Aspire.Dashboard/wwwroot/js/app-theme.js Show resolved Hide resolved

tlmii added 3 commits July 16, 2024 10:00

Create hover tokens

e881898

CR Feedback

789c287

Add back pointer to trace detail page

cc0b80e

tlmii force-pushed the dev/timulhol/hover-tokens branch from fdf07d5 to cc0b80e Compare July 16, 2024 17:00

build-analysis bot mentioned this pull request Jul 16, 2024

Failing test Aspire.Hosting.Kafka.Tests.KafkaFunctionalTests.VerifyKafkaResource - Broker: Unknown topic or partition #4929

Closed

JamesNK approved these changes Jul 16, 2024

View reviewed changes

Update workload tests for new markup

1e120b2

tlmii requested review from radical and eerhardt as code owners July 17, 2024 05:31

JamesNK approved these changes Jul 17, 2024

View reviewed changes

JamesNK mentioned this pull request Jul 17, 2024

Improve CheckDashboardHasResourcesAsync in workload tests #4834

Open

1 task

tlmii merged commit 12ad5c3 into dotnet:main Jul 17, 2024
9 checks passed

github-actions bot locked and limited conversation to collaborators Aug 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create hover tokens for grid row hover effect #4892

Create hover tokens for grid row hover effect #4892

tlmii commented Jul 15, 2024 •

edited by dotnet-policy-service bot

Loading

JamesNK left a comment •

edited

Loading

tlmii commented Jul 16, 2024

vnbaaij commented Jul 16, 2024

JamesNK commented Jul 16, 2024

JamesNK commented Jul 16, 2024 •

edited

Loading

tlmii commented Jul 16, 2024

tlmii commented Jul 16, 2024

azure-pipelines bot commented Jul 16, 2024

JamesNK commented Jul 16, 2024

JamesNK Jul 17, 2024 •

edited

Loading

tlmii Jul 17, 2024

JamesNK Jul 17, 2024

Create hover tokens for grid row hover effect #4892

Create hover tokens for grid row hover effect #4892

Conversation

tlmii commented Jul 15, 2024 • edited by dotnet-policy-service bot Loading

Microsoft Reviewers: Open in CodeFlow

JamesNK left a comment • edited Loading

Choose a reason for hiding this comment

tlmii commented Jul 16, 2024

vnbaaij commented Jul 16, 2024

JamesNK commented Jul 16, 2024

JamesNK commented Jul 16, 2024 • edited Loading

tlmii commented Jul 16, 2024

tlmii commented Jul 16, 2024

azure-pipelines bot commented Jul 16, 2024

JamesNK commented Jul 16, 2024

JamesNK Jul 17, 2024 • edited Loading

Choose a reason for hiding this comment

tlmii Jul 17, 2024

Choose a reason for hiding this comment

JamesNK Jul 17, 2024

Choose a reason for hiding this comment

tlmii commented Jul 15, 2024 •

edited by dotnet-policy-service bot

Loading

JamesNK left a comment •

edited

Loading

JamesNK commented Jul 16, 2024 •

edited

Loading

JamesNK Jul 17, 2024 •

edited

Loading