Skip to content

Make opening visualizer the default grid value action#6900

Merged
JamesNK merged 5 commits intomainfrom
jamesnk/visualizer-open
Dec 11, 2024
Merged

Make opening visualizer the default grid value action#6900
JamesNK merged 5 commits intomainfrom
jamesnk/visualizer-open

Conversation

@JamesNK
Copy link
Copy Markdown
Member

@JamesNK JamesNK commented Dec 9, 2024

Description

This PR:

  • Replaces grid value menu (has copy and launch visualizer options) with a button that just launches visualizer. The visualizer has a copy button so folks can copy from the dialog.
  • Changes visualizer to be launched server side. The previous system seemed very complex for no value. Previous steps: adding attributes to an HTML element, listening for the elements in a document click event, triggering a server method and streaming the element values back to the server, then showing the dialog. @adamint Is there a reason it was implemented this way?
  • Changes visualizer launch icon to the same icon displayed in the visualizer
  • Fixes masking spacing issue

Fixes #6685

visualizer-launch

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?
Microsoft Reviewers: Open in CodeFlow

Copy link
Copy Markdown
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.

Copilot reviewed 5 out of 14 changed files in this pull request and generated no suggestions.

Files not reviewed (9)
  • src/Aspire.Dashboard/Components/Controls/GridValue.razor.css: Language not supported
  • src/Aspire.Dashboard/Components/Pages/Resources.razor: Evaluated as low risk
  • src/Aspire.Dashboard/Components/Controls/GridValue.razor: Evaluated as low risk
  • src/Aspire.Dashboard/Components/Controls/PropertyGrid.razor: Evaluated as low risk
  • src/Aspire.Dashboard/Components/ResourcesGridColumns/SourceColumnDisplay.razor: Evaluated as low risk
  • src/Aspire.Dashboard/Components/Controls/Chart/PlotlyChart.razor.cs: Evaluated as low risk
  • src/Aspire.Dashboard/wwwroot/js/app.js: Evaluated as low risk
  • tests/Aspire.Dashboard.Tests/Model/ResourceSourceViewModelTests.cs: Evaluated as low risk
  • src/Aspire.Dashboard/Extensions/FluentUIExtensions.cs: Evaluated as low risk
Comments skipped due to low confidence (1)

src/Aspire.Dashboard/Components/Layout/MainLayout.razor.cs:23

  • Consider adding a check to see if _isNavMenuOpen is already false before calling StateHasChanged() to avoid unnecessary state updates.
private void CloseMobileNavMenu()

@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Dec 9, 2024

UI change based on customer feedback. Feels like a good improvement.

cc @leslierichardson95 @DamianEdwards @davidfowl

@DamianEdwards
Copy link
Copy Markdown
Member

@JamesNK yeah this looks like a good improvement! 👍

@JamesNK JamesNK merged commit fedc205 into main Dec 11, 2024
@JamesNK JamesNK deleted the jamesnk/visualizer-open branch December 11, 2024 00:53
@github-actions github-actions Bot locked and limited conversation to collaborators Jan 10, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider buttons displayed next to values in dashboard

4 participants