Repository navigation
Plan viewer: a finding's header links to the operator it came from (#4534) - #4559
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #4534
Part of #4511
Why
A plan-level or per-operator warning knows which operator it came from (
PlanWarning.OriginNodeIds, landed in #4552/#4556), but the viewer's warning headers didn't expose it. A reader had to already know which operator to look at.What changes
PerformanceMonitor.PlanAnalysis/PlanWarningDisplay.cs: newOriginNavigationText(IReadOnlyList<int> originNodeIds)returns(Suffix, Tooltip)?— null when the list is empty (no affordance), otherwise a" →"suffix and a tooltip naming the operator ("Go to operator (Node 7)"), or for several origins, the first plus the rest ("Go to Node 7 — also from Node 9, Node 12").PerformanceMonitor.Ui/PlanViewerControl.Rendering.cs: newAttachOriginNavigation(TextBlock header, string headerText, List<int> originNodeIds)appends the suffix, sets a hand cursor and a transparent background (so the whole line hit-tests, not just the glyphs), attaches the tooltip, and wiresMouseLeftButtonDownto navigate to the first origin node.PerformanceMonitor.Ui/PlanViewerControl.Properties.cs: both the plan-level and the per-operator warning header sites now callAttachOriginNavigation.PerformanceMonitor.Ui/PlanViewerControl.Interaction.cs: newTryNavigateToNode(int nodeId)finds the renderedBordertagged with thatNodeIdamongPlanCanvas.Children, selects it, and scrolls it into view (newScrollNodeIntoView, ported from the same navigation feature, using WPF'sScrollVieweroffset/extent/viewport members). Returns false when no operator with that id is rendered, so a stale or wrong origin doesn't scroll to something arbitrary.This mirrors PerformanceStudio's
AttachOriginNavigation/TryNavigateToNode(AvaloniaCursor/ToolTip.SetTip/PointerPressed), translated to WPF'sCursors.Hand/ToolTipService/MouseLeftButtonDown.Both the Darling viewer and Lite host the same shared
PerformanceMonitor.Ui.PlanViewerControl(Darling/PerformanceMonitor.Darling.Viewer/MainWindow.PlanViewer.cs,ProcedureHistoryWindow.xaml.cs), so they both get this navigation for free — no separate change needed there.Test plan
New class
Darling.Tests.PlanViewerOriginNavigationTests:OriginNavigationTextreturns null;" →", tooltip"Go to operator (Node 7)";"Go to Node 7 — also from Node 9, Node 12".The WPF wiring (
AttachOriginNavigation,TryNavigateToNode,ScrollNodeIntoView) can't run on macOS. Traced by hand:TryNavigateToNode(nodeId)walksPlanCanvas.Children, each of which is aBorderwhoseTagis set to itsPlanNodeinCreateNodeVisual(PlanViewerControl.Rendering.cs,Tag = node) — the same taggingNode_Click/SelectNodealready rely on for click-to-select. For a real, renderedNodeIdthis loop finds the matching border, calls the existingSelectNode(used by direct clicks too), andScrollNodeIntoView, which multiplies the node's unscaledX/Yby_zoomLeveland clamps the offset to[0, Extent - Viewport]. For aNodeIdnot present (stale data) the loop falls through and returns false, and no click handler is invoked.OriginNavigationTextis new). Mutation: returning an affordance for an empty list fails the no-IDs fact.Total: 237, Failed: 0.PerformanceMonitor.Ui,LiteandDarling.Testsbuild with 0 warnings. The Darling viewer and Lite both use this sharedPlanViewerControl, so both get the navigation.CHANGELOG
SECTION: Added
ENTRY: - Plan viewer: a finding's header links to the operator it came from ([#4559]) - Plan-level and per-operator warnings that know which operator they came from now show a small arrow in their header; clicking it selects and scrolls to that operator.
REF: [#4559]: #4559