Skip to content

implements #225#226

Merged
erikdarlingdata merged 4 commits intoerikdarlingdata:devfrom
ClaudioESSilva:feature/auto-scrolling
Apr 14, 2026
Merged

implements #225#226
erikdarlingdata merged 4 commits intoerikdarlingdata:devfrom
ClaudioESSilva:feature/auto-scrolling

Conversation

@ClaudioESSilva
Copy link
Copy Markdown
Contributor

What does this PR do?

Implements #225

Which component(s) does this affect?

  • Desktop App (PlanViewer.App)
  • Core Library (PlanViewer.Core)
  • CLI Tool (PlanViewer.Cli)
  • SSMS Extension (PlanViewer.Ssms)
  • Tests
  • Documentation

How was this tested?

  1. Connect to Query Store
  2. Use scroll-wheel click to navigate the results grid

Check the video on #225

Checklist

  • I have read the contributing guide
  • My code builds with zero warnings (dotnet build -c Debug)
  • All tests pass (dotnet test)
  • I have not introduced any hardcoded credentials or server names

Copy link
Copy Markdown
Owner

@erikdarlingdata erikdarlingdata left a comment

Choose a reason for hiding this comment

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

Nice work — clean separation with the DataGridBehaviors helper, and handling Avalonia's scrollbar quirks correctly. A few things I noticed:

1. Reflection on internal methods (fragile)

ProcessHorizontalScroll / ProcessVerticalScroll are private Avalonia internals accessed via reflection. If Avalonia renames or removes them in a future version, this breaks silently — the scrollbar Value would update but the grid content wouldn't visually scroll. The null checks degrade gracefully, but worth calling out as a maintenance risk.

2. Scroll direction may be inverted

The delta uses scrollStartH + delta.X, which moves the scrollbar in the same direction as the mouse. Typical pan behavior drags the content, not the scrollbar thumb — dragging right should reveal content to the left. You might want to negate the deltas:

hBar.Value = Math.Clamp(scrollStartH - delta.X, hBar.Minimum, hBar.Maximum);
// ...
vBar.Value = Math.Clamp(scrollStartV - delta.Y, vBar.Minimum, vBar.Maximum);

Test both directions and see which feels more natural. Browser and PDF viewer pan behavior uses the negated version.

3. Opened change is unrelated

The MainWindow change deferring plan loading to the Opened event is a separate fix. Not a blocker, but ideally would be its own commit so bisecting is easier if something regresses.

@ClaudioESSilva
Copy link
Copy Markdown
Contributor Author

@erikdarlingdata all (3) changes done.

Copy link
Copy Markdown
Owner

@erikdarlingdata erikdarlingdata left a comment

Choose a reason for hiding this comment

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

All three items addressed cleanly — reflection replaced with public Thumb.DragDeltaEvent, scroll direction fixed, and the Opened change separated out. Thanks for the contribution and for the quick turnaround on the feedback, @ClaudioESSilva!

@erikdarlingdata erikdarlingdata merged commit be5c351 into erikdarlingdata:dev Apr 14, 2026
2 checks passed
@erikdarlingdata erikdarlingdata mentioned this pull request Apr 15, 2026
3 tasks
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