Refactor ChartCanvas: replace getTooltipPosition/viewportTop with selectedItemTooltipOffset

Replace the getTooltipPosition callback and viewportTop props with a single pre-computed selectedItemTooltipOffset prop. The parent component (MarkerChartCanvas) now owns the full tooltip position calculation including the viewport bounds check, since it has direct access to all the layout data.
This simplifies ChartCanvas by removing internal details it didn't need to know about:
 - _syncSelectedItemFromProp no longer calls a callback or checks viewport bounds;
 - The three separate componentDidUpdate trigger conditions collapse into one;
 - viewportTop is no longer leaked as a prop.

Also fix scroll-to-selected-marker on load when the viewport resizes after the initial layout (e.g. flex containers settling), by re-running _scrollSelectionIntoView when containerHeight changes.

Author: fatadel

src/components/marker-chart/Canvas.tsx

@@ -148,14 +148,16 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
   _textMeasurement: TextMeasurement | null = null;
 
   override componentDidUpdate(prevProps: Props) {
-    // When the viewport finishes sizing itself, or when the selected marker changes,
-    // scroll to bring the selected marker into view (e.g. on initial load from URL).
     const viewportDidMount =
       !prevProps.viewport.isSizeSet && this.props.viewport.isSizeSet;
+    const viewportResized =
+      this.props.viewport.isSizeSet &&
+      this.props.viewport.containerHeight !==
+        prevProps.viewport.containerHeight;
     const selectedMarkerChanged =
       this.props.selectedMarkerIndex !== prevProps.selectedMarkerIndex;
 
-    if (viewportDidMount || selectedMarkerChanged) {
+    if (viewportDidMount || viewportResized || selectedMarkerChanged) {
       this._scrollSelectionIntoView();
     }
   }
@@ -997,31 +999,40 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
   };
 
   /**
-   * Calculate the canvas position for a marker's tooltip based on the marker's index.
-   * The method is used when the selected marker comes from the URL and does NOT
-   * necessarily correspond to what's being hovered at the moment.
-   *
-   * @param markerIndex - The marker's index in the original thread's marker table
-   * @returns Canvas-relative coordinates {offsetX, offsetY} or null if marker not visible
+   * Compute canvas-relative tooltip offset for the selected marker.
+   * Returns null if the marker can't be located in the timing data or
+   * is outside the visible viewport.
    */
-  getTooltipPosition = (
-    markerIndex: MarkerIndex
-  ): { offsetX: CssPixels; offsetY: CssPixels } | null => {
+  _getSelectedItemTooltipOffset(): {
+    offsetX: CssPixels;
+    offsetY: CssPixels;
+  } | null {
+    const { selectedMarkerIndex } = this.props;
+    if (selectedMarkerIndex === null) {
+      return null;
+    }
+
     const {
       rangeStart,
       rangeEnd,
       markerTimingAndBuckets,
       rowHeight,
       marginLeft,
       marginRight,
-      viewport: { containerWidth, viewportLeft, viewportRight, viewportTop },
+      viewport: {
+        containerWidth,
+        containerHeight,
+        viewportLeft,
+        viewportRight,
+        viewportTop,
+      },
     } = this.props;
 
     // Step 1: Find which row this marker is displayed in
     const markerIndexToTimingRow = this._getMarkerIndexToTimingRow(
       markerTimingAndBuckets
     );
-    const rowIndex = markerIndexToTimingRow[markerIndex];
+    const rowIndex = markerIndexToTimingRow[selectedMarkerIndex];
 
     // Step 2: Get the timing data for all markers in this row
     const markerTiming = markerTimingAndBuckets[rowIndex];
@@ -1033,22 +1044,20 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
     // Step 3: Find the position of our specific marker within this row's data
     let markerTimingIndex = -1;
     for (let i = 0; i < markerTiming.length; i++) {
-      if (markerTiming.index[i] === markerIndex) {
+      if (markerTiming.index[i] === selectedMarkerIndex) {
         markerTimingIndex = i;
         break;
       }
     }
 
     if (markerTimingIndex === -1) {
-      // Marker not found in this row's data (shouldn't happen, but handle gracefully)
       return null;
     }
 
     // Step 4: Calculate horizontal (X) position
     const startTimestamp = markerTiming.start[markerTimingIndex];
     const endTimestamp = markerTiming.end[markerTimingIndex];
 
-    // Convert absolute timestamps to relative positions (0.0 to 1.0 of the full range)
     const markerContainerWidth = containerWidth - marginLeft - marginRight;
     const rangeLength: Milliseconds = rangeEnd - rangeStart;
     const viewportLength: UnitIntervalOfProfileRange =
@@ -1058,29 +1067,29 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
     const endTime: UnitIntervalOfProfileRange =
       (endTimestamp - rangeStart) / rangeLength;
 
-    // Calculate pixel position: map the time range to the visible viewport
     const x: CssPixels =
       ((startTime - viewportLeft) * markerContainerWidth) / viewportLength +
       marginLeft;
     const w: CssPixels =
       ((endTime - startTime) * markerContainerWidth) / viewportLength;
 
-    // For instant markers (start === end), use the center point
-    // For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller)
+    // For instant markers (start === end), use the center point.
+    // For interval markers, use a point 1/3 into the marker (or 30px, whichever is smaller).
     const isInstantMarker = startTimestamp === endTimestamp;
     const offsetX = isInstantMarker ? x : x + Math.min(w / 3, 30);
 
     // Step 5: Calculate vertical (Y) position
-    // Place the tooltip at the top of the marker's row, with a small offset
     const offsetY: CssPixels = rowIndex * rowHeight - viewportTop + 5;
 
-    // Return canvas-relative coordinates (should be converted to page coordinates by caller)
+    if (offsetY < 0 || offsetY > containerHeight) {
+      return null;
+    }
+
     return { offsetX, offsetY };
-  };
+  }
 
   override render() {
-    const { containerWidth, containerHeight, isDragging, viewportTop } =
-      this.props.viewport;
+    const { containerWidth, containerHeight, isDragging } = this.props.viewport;
     const { selectedMarkerIndex } = this.props;
 
     return (
@@ -1100,8 +1109,7 @@ class MarkerChartCanvasImpl extends React.PureComponent<Props> {
         onMouseLeave={this.onMouseLeave}
         stickyTooltips={true}
         selectedItem={selectedMarkerIndex}
-        getTooltipPosition={this.getTooltipPosition}
-        viewportTop={viewportTop}
+        selectedItemTooltipOffset={this._getSelectedItemTooltipOffset()}
       />
     );
   }

src/components/shared/chart/Canvas.tsx

@@ -35,12 +35,12 @@ type Props<Item> = {
   readonly stickyTooltips?: boolean;
   // Selected item coming from URL or other non-click mechanism.
   readonly selectedItem?: Item | null;
-  // Method to compute tooltip position for the selected item above.
-  readonly getTooltipPosition?: (
-    item: Item
-  ) => { offsetX: CssPixels; offsetY: CssPixels } | null;
-  // Current vertical scroll offset of the viewport.
-  readonly viewportTop?: CssPixels;
+  // Pre-computed canvas-relative offset for the tooltip of the selected item
+  // (null when the item is not currently visible in the viewport).
+  readonly selectedItemTooltipOffset?: {
+    offsetX: CssPixels;
+    offsetY: CssPixels;
+  } | null;
 };
 
 // The naming of the X and Y coordinates here correspond to the ones
@@ -367,51 +367,36 @@ export class ChartCanvas<Item> extends React.Component<
     this._canvas = canvas;
   };
 
-  _syncSelectedItemFromProp = (selectedItem: Item | null) => {
-    if (selectedItem === null) {
-      this.setState({ selectedItem: null });
-      return;
-    }
+  _syncSelectedItemFromProp = () => {
+    const { selectedItem, selectedItemTooltipOffset } = this.props;
 
-    if (!this.props.getTooltipPosition || !this._canvas) {
+    if (selectedItem === undefined || selectedItem === null) {
+      this.setState({ selectedItem: null });
       return;
     }
 
-    const tooltipPosition = this.props.getTooltipPosition(selectedItem);
-    if (!tooltipPosition) {
-      // Can't get position, but still set the selectedItem
+    if (!selectedItemTooltipOffset || !this._canvas) {
+      // Keep selectedItem set, so the tooltip stays at
+      // its last known position rather than disappearing.
       this.setState({ selectedItem });
       return;
     }
 
-    const { offsetX, offsetY } = tooltipPosition;
-
-    // If the item is outside the visible canvas area, skip setting the tooltip
-    // position for now. The viewport will scroll to bring it into view, and the
-    // viewportTop change will trigger a re-sync with the correct position.
-    if (offsetY < 0 || offsetY > this.props.containerHeight) {
-      return;
-    }
-
+    const { offsetX, offsetY } = selectedItemTooltipOffset;
     const canvasRect = this._canvas.getBoundingClientRect();
     const pageX = canvasRect.left + window.scrollX + offsetX;
     const pageY = canvasRect.top + window.scrollY + offsetY;
-    this.setState({
-      selectedItem,
-      pageX,
-      pageY,
-    });
+    this.setState({ selectedItem, pageX, pageY });
   };
 
   override componentDidMount() {
-    // Initialize selectedItem from props on mount if provided and the canvas
-    // already has dimensions. If containerWidth is still 0, the viewport hasn't
-    // been laid out yet - componentDidUpdate will pick it up once it becomes non-zero.
+    // If the viewport hasn't been laid out yet,
+    // componentDidUpdate will pick it up once it becomes non-zero.
     if (
       this.props.selectedItem !== undefined &&
       this.props.containerWidth !== 0
     ) {
-      this._syncSelectedItemFromProp(this.props.selectedItem);
+      this._syncSelectedItemFromProp();
     }
   }
 
@@ -433,35 +418,33 @@ export class ChartCanvas<Item> extends React.Component<
 
   override componentDidUpdate(prevProps: Props<Item>, prevState: State<Item>) {
     if (prevProps !== this.props) {
-      // Sync selectedItem state with selectedItem prop if it changed
-      // and the state doesn't already have this item selected.
-      if (
-        this.props.selectedItem !== undefined &&
-        this.props.selectedItem !== prevProps.selectedItem &&
-        !hoveredItemsAreEqual(this.state.selectedItem, this.props.selectedItem)
-      ) {
-        this._syncSelectedItemFromProp(this.props.selectedItem);
-      }
-
-      // The canvas just received its dimensions for the first time (containerWidth
-      // went from 0 to non-zero). If a selectedItem was provided but couldn't be
-      // synced in componentDidMount, do it now.
-      if (
-        prevProps.containerWidth === 0 &&
-        this.props.containerWidth !== 0 &&
-        this.props.selectedItem !== undefined
-      ) {
-        this._syncSelectedItemFromProp(this.props.selectedItem);
-      }
-
-      // The viewport scrolled (e.g. to bring the selected item into view on
-      // load). Re-sync the tooltip position so it stays on the selected item.
-      if (
-        this.props.viewportTop !== undefined &&
-        this.props.viewportTop !== prevProps.viewportTop &&
-        this.props.selectedItem !== undefined
-      ) {
-        this._syncSelectedItemFromProp(this.props.selectedItem);
+      if (this.props.selectedItem !== undefined) {
+        const selectedItemChanged =
+          this.props.selectedItem !== prevProps.selectedItem;
+        const canvasJustGotSize =
+          prevProps.containerWidth === 0 && this.props.containerWidth !== 0;
+        const offsetChanged =
+          this.props.selectedItemTooltipOffset !==
+          prevProps.selectedItemTooltipOffset;
+
+        const alreadySetInternally =
+          selectedItemChanged &&
+          hoveredItemsAreEqual(
+            this.state.selectedItem,
+            this.props.selectedItem
+          );
+
+        const offsetSyncRelevant =
+          offsetChanged &&
+          !selectedItemChanged &&
+          this.state.selectedItem !== null;
+
+        if (
+          !alreadySetInternally &&
+          (selectedItemChanged || canvasJustGotSize || offsetSyncRelevant)
+        ) {
+          this._syncSelectedItemFromProp();
+        }
       }
 
       if (