Skip to content

Reset details scroll position#7040

Merged
JamesNK merged 3 commits intomainfrom
jamesnk/details-scrolltop
Jan 10, 2025
Merged

Reset details scroll position#7040
JamesNK merged 3 commits intomainfrom
jamesnk/details-scrolltop

Conversation

@JamesNK
Copy link
Copy Markdown
Member

@JamesNK JamesNK commented Jan 8, 2025

Description

Switching details item while the details view is open (e.g. clicking between resources on resources page) doesn't reset the scroll position. The reason this happens is Blazor rerenders the content of the scroll container, but leaves the container unchanged, preserving scroll position.

The user impact of this bug is if you view a resource's details, scroll down in the details, then click another resource, the other resource is also scrolled. This bug also impacts log and span details.

This PR tracks when the data has changed, and invokes JS to reset the details view scrollbar to the top after render.

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?

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 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (2)

src/Aspire.Dashboard/Components/Controls/StructuredLogDetails.razor.cs:99

  • The word 'desintation' is misspelled. It should be 'destination'.
private static void MoveAttributes(List<TelemetryPropertyViewModel> source, List<TelemetryPropertyViewModel> desintation, Func<TelemetryPropertyViewModel, bool> predicate)

src/Aspire.Dashboard/Components/Controls/StructuredLogDetails.razor.cs:86

  • The new behavior introduced in the OnAfterRenderAsync method should be covered by tests to ensure it works as expected.
protected override async Task OnAfterRenderAsync(bool firstRender)

@JamesNK
Copy link
Copy Markdown
Member Author

JamesNK commented Jan 8, 2025

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@JamesNK JamesNK force-pushed the jamesnk/details-scrolltop branch from e1d004a to d9a6133 Compare January 9, 2025 22:46
@JamesNK JamesNK enabled auto-merge (squash) January 9, 2025 22:46
@JamesNK JamesNK merged commit b6de754 into main Jan 10, 2025
@JamesNK JamesNK deleted the jamesnk/details-scrolltop branch January 10, 2025 02:01
@github-actions github-actions Bot locked and limited conversation to collaborators Feb 9, 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.

3 participants