Skip to content

Conversation

@mcumming
Copy link

@mcumming mcumming commented Oct 24, 2025

Description

Added IconName, IconVarient to ResourceUrlAnnotation to display urls using Fluent Icons.

image

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?

Added IconName, IconVarient to ResourceUrlAnnotaion and plumbed through.
Copilot AI review requested due to automatic review settings October 24, 2025 13:54
@github-actions
Copy link
Contributor

github-actions bot commented Oct 24, 2025

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 12346

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 12346"

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for displaying URLs with FluentUI icons in the Aspire Dashboard by introducing IconName and IconVariant properties to ResourceUrlAnnotation. This enhancement allows resources to display custom icons alongside their URLs, improving visual identification in the dashboard UI.

Key changes:

  • Added IconName and IconVariant properties to ResourceUrlAnnotation class
  • Updated protobuf definitions and snapshot records to include icon properties
  • Modified dashboard components to render URLs with icons using FluentButton when icon data is present

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
src/Aspire.Hosting/ApplicationModel/ResourceUrlAnnotation.cs Added IconName and IconVariant properties to store icon metadata
src/Aspire.Hosting/ApplicationModel/CustomResourceSnapshot.cs Extended UrlDisplayPropertiesSnapshot record to include icon parameters
src/Aspire.Hosting/Dashboard/proto/dashboard_service.proto Added optional icon_name and icon_variant fields to UrlDisplayProperties message
src/Aspire.Hosting/Dashboard/proto/Partials.cs Added logic to map icon properties from snapshot to protobuf message
src/Aspire.Hosting/Orchestrator/ApplicationOrchestrator.cs Updated snapshot creation to pass icon properties
src/Aspire.Hosting/Dcp/ResourceSnapshotBuilder.cs Updated URL snapshot creation to include icon properties
src/Aspire.Dashboard/ServiceClient/Partials.cs Added MapIconVariant helper and updated UrlViewModel construction
src/Aspire.Dashboard/Model/ResourceViewModel.cs Updated UrlDisplayPropertiesViewModel to include icon properties
src/Aspire.Dashboard/Model/ResourceUrlHelpers.cs Extended DisplayedUrl model with icon properties
src/Aspire.Dashboard/Components/ResourcesGridColumns/UrlsColumnDisplay.razor.cs Added required service injections for icon rendering
src/Aspire.Dashboard/Components/ResourcesGridColumns/UrlsColumnDisplay.razor Updated UI to render FluentButton with icon when icon data is available
playground/TestShop/TestShop.AppHost/Program.cs Added example usage demonstrating icon configuration for PG Admin URL

Endpoint = endpoint,
DisplayOrder = DisplayOrder,
DisplayLocation = DisplayLocation
DisplayLocation = DisplayLocation,
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The WithEndpoint method does not copy the IconName and IconVariant properties to the new annotation instance. These properties should be included in the object initializer to preserve icon settings when creating endpoint-specific annotations.

Suggested change
DisplayLocation = DisplayLocation,
DisplayLocation = DisplayLocation,
IconName = IconName,
IconVariant = IconVariant,

Copilot uses AI. Check for mistakes.
<FluentIcon Value="@icon" Width="16px" Title="@displayedUrl.Text" />
</FluentButton>;
}
//<a href="@displayedUrl.Url" target="_blank" @onclick:stopPropagation="true">
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Remove commented-out code. If this was left for reference during development, it should be deleted before merging.

Suggested change
//<a href="@displayedUrl.Url" target="_blank" @onclick:stopPropagation="true">

Copilot uses AI. Check for mistakes.
public sealed record UrlDisplayPropertiesSnapshot(string DisplayName = "", int SortOrder = 0);
/// <param name="IconName">The icon name for the Url. The name should be a valid FluentUI icon name. https://aka.ms/fluentui-system-icons</param>
/// <param name="IconVariant">The icon variant.</param>
///
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Remove extra blank line in XML documentation comment. This line serves no purpose and should be deleted.

Suggested change
///

Copilot uses AI. Check for mistakes.
public record UrlDisplayPropertiesViewModel(string DisplayName, int SortOrder, string? IconName, IconVariant? IconVariant)
{
public static readonly UrlDisplayPropertiesViewModel Empty = new(string.Empty, 0);
public static readonly UrlDisplayPropertiesViewModel Empty = new(string.Empty, 0, string.Empty, (IconVariant)(-1));
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Using (IconVariant)(-1) as a sentinel value is fragile and could cause issues if IconVariant enum changes. Consider using null since IconVariant is already nullable in the constructor, or define a proper sentinel constant like IconVariant.None.

Suggested change
public static readonly UrlDisplayPropertiesViewModel Empty = new(string.Empty, 0, string.Empty, (IconVariant)(-1));
public static readonly UrlDisplayPropertiesViewModel Empty = new(string.Empty, 0, string.Empty, null);

Copilot uses AI. Check for mistakes.
{
if (string.IsNullOrWhiteSpace(displayedUrl.IconName) == false && IconResolver.ResolveIconName(displayedUrl.IconName, IconSize.Size16, iconVariant: displayedUrl.IconVariant) is { } icon)
{
return @<FluentButton Appearance="Appearance.Lightweight" Title="@displayedUrl.Text" StopPropagation="true" OnClick="@(() => JSRuntime.InvokeAsync<object>("open", displayedUrl.Url, "_blank"))" >
Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

The FluentButton lacks accessible text content for screen readers. When displaying only an icon, add an aria-label attribute or use the AriaLabel parameter on FluentButton to provide accessible text equivalent to the Title attribute.

Suggested change
return @<FluentButton Appearance="Appearance.Lightweight" Title="@displayedUrl.Text" StopPropagation="true" OnClick="@(() => JSRuntime.InvokeAsync<object>("open", displayedUrl.Url, "_blank"))" >
return @<FluentButton Appearance="Appearance.Lightweight" Title="@displayedUrl.Text" AriaLabel="@displayedUrl.Text" StopPropagation="true" OnClick="@(() => JSRuntime.InvokeAsync<object>("open", displayedUrl.Url, "_blank"))" >

Copilot uses AI. Check for mistakes.
{
displayProperties.IconVariant = MapIconVariant(urlSnapshot.DisplayProperties.IconVariant);
}

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Remove trailing whitespace at end of line.

Suggested change

Copilot uses AI. Check for mistakes.

[Inject]
public required IJSRuntime JSRuntime { get; init; }

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Remove trailing whitespace at end of line.

Suggested change

Copilot uses AI. Check for mistakes.
/// The name of the icon to display with this URL.
/// </summary>
public string IconName { get; set; } = string.Empty;

Copy link

Copilot AI Oct 24, 2025

Choose a reason for hiding this comment

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

Remove trailing whitespace at end of line.

Suggested change

Copilot uses AI. Check for mistakes.
Copy link
Member

@adamint adamint left a comment

Choose a reason for hiding this comment

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

Can I ask what prompted this request?

Ideally, I would like to bias towards including text in any button, especially if it contains user-defined content where the meaning of the icon may not be immediately obvious.

Would it fit your needs to instead, allow showing icons after the url text? Or if you do need this behavior, I do think it should be disabled in the url dropdown and only visible inline in the endpoints cell.

…riants; enhance FluentButton accessibility with AriaLabel
@mcumming
Copy link
Author

Can I ask what prompted this request?

Ideally, I would like to bias towards including text in any button, especially if it contains user-defined content where the meaning of the icon may not be immediately obvious.

Would it fit your needs to instead, allow showing icons after the url text? Or if you do need this behavior, I do think it should be disabled in the url dropdown and only visible inline in the endpoints cell.

Thanks for the feedback! This was something I thought of while I was working on some extensions for another project. I thought having the ability to replace the (long) url with an icon like Actions would be a nice option. When I asked @maddymontaquila about it she thought it was a good idea so I started mocking it up, then was told ship it by @maddymontaquila. So that's why the PR is here.

I played with adding the Text to the button but was challenged by getting all the alignments working correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants