DevTools: Scheduling profiler: Add vertical scroll bar (#22005)

Co-authored-by: E-Liang Tan <[email protected]>

Author: Brian Vaughn

packages/react-devtools-scheduling-profiler/src/CanvasPage.js

@@ -30,14 +30,12 @@ import {copy} from 'clipboard-js';
 import prettyMilliseconds from 'pretty-ms';
 
 import {
-  BackgroundColorView,
   HorizontalPanAndZoomView,
   ResizableView,
+  VerticalScrollOverflowView,
   Surface,
   VerticalScrollView,
   View,
-  createComposedLayout,
-  lastViewTakesUpRemainingSpaceLayout,
   useCanvasInteraction,
   verticallyStackedLayout,
   zeroPoint,
@@ -325,10 +323,9 @@ function AutoSizedCanvas({
     const rootView = new View(
       surface,
       defaultFrame,
-      createComposedLayout(
-        verticallyStackedLayout,
-        lastViewTakesUpRemainingSpaceLayout,
-      ),
+      verticallyStackedLayout,
+      defaultFrame,
+      COLORS.BACKGROUND,
     );
     rootView.addSubview(axisMarkersViewWrapper);
     if (userTimingMarksViewWrapper !== null) {
@@ -345,10 +342,14 @@ function AutoSizedCanvas({
     }
     rootView.addSubview(flamechartViewWrapper);
 
-    // If subviews are less than the available height, fill remaining height with a solid color.
-    rootView.addSubview(new BackgroundColorView(surface, defaultFrame));
+    const verticalScrollOverflowView = new VerticalScrollOverflowView(
+      surface,
+      defaultFrame,
+      rootView,
+      viewState,
+    );
 
-    surfaceRef.current.rootView = rootView;
+    surfaceRef.current.rootView = verticalScrollOverflowView;
   }, [data]);
 
   useLayoutEffect(() => {
@@ -401,6 +402,16 @@ function AutoSizedCanvas({
     const surface = surfaceRef.current;
     surface.handleInteraction(interaction);
 
+    // Flush any display work that got queued up as part of the previous interaction.
+    // Typically there should be no work, but certain interactions may need a second pass.
+    // For example, the ResizableView may collapse/expand its contents,
+    // which requires a second layout pass for an ancestor VerticalScrollOverflowView.
+    //
+    // TODO It would be nice to remove this call for performance reasons.
+    // To do that, we'll need to address the UX bug with VerticalScrollOverflowView.
+    // For more info see: https://github.com/facebook/react/pull/22005#issuecomment-896953399
+    surface.displayIfNeeded();
+
     canvas.style.cursor = surface.getCurrentCursor() || 'default';
 
     // Defer drawing to canvas until React's commit phase, to avoid drawing

packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js

@@ -241,9 +241,11 @@ function processTimelineEvent(
 
         // Reduce noise from events like DOMActivate, load/unload, etc. which are usually not relevant
         if (
-          type.startsWith('blur') ||
-          type.startsWith('click') ||
+          type === 'blur' ||
+          type === 'click' ||
+          type === 'input' ||
           type.startsWith('focus') ||
+          type.startsWith('key') ||
           type.startsWith('mouse') ||
           type.startsWith('pointer')
         ) {

packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollOverflowView.js

@@ -34,10 +34,16 @@ const CARET_MARGIN = 3;
 const CARET_WIDTH = 5;
 const CARET_HEIGHT = 3;
 
+type OnChangeCallback = (
+  scrollState: ScrollState,
+  containerLength: number,
+) => void;
+
 export class VerticalScrollView extends View {
   _contentView: View;
   _isPanning: boolean;
   _mutableViewStateKey: string;
+  _onChangeCallback: OnChangeCallback | null;
   _scrollState: ScrollState;
   _viewState: ViewState;
 
@@ -53,6 +59,7 @@ export class VerticalScrollView extends View {
     this._contentView = contentView;
     this._isPanning = false;
     this._mutableViewStateKey = label + ':VerticalScrollView';
+    this._onChangeCallback = null;
     this._scrollState = {
       offset: 0,
       length: 0,
@@ -146,26 +153,45 @@ export class VerticalScrollView extends View {
     super.layoutSubviews();
   }
 
-  handleInteraction(interaction: Interaction) {
+  handleInteraction(interaction: Interaction): ?boolean {
     switch (interaction.type) {
       case 'mousedown':
-        this._handleMouseDown(interaction);
-        break;
+        return this._handleMouseDown(interaction);
       case 'mousemove':
-        this._handleMouseMove(interaction);
-        break;
+        return this._handleMouseMove(interaction);
       case 'mouseup':
-        this._handleMouseUp(interaction);
-        break;
+        return this._handleMouseUp(interaction);
       case 'wheel-shift':
-        this._handleWheelShift(interaction);
-        break;
+        return this._handleWheelShift(interaction);
     }
   }
 
+  onChange(callback: OnChangeCallback) {
+    this._onChangeCallback = callback;
+  }
+
+  scrollBy(deltaY: number): boolean {
+    const newState = translateState({
+      state: this._scrollState,
+      delta: -deltaY,
+      containerLength: this.frame.size.height,
+    });
+
+    // If the state is updated by this wheel scroll,
+    // return true to prevent the interaction from bubbling.
+    // For instance, this prevents the outermost container from also scrolling.
+    return this._setScrollState(newState);
+  }
+
   _handleMouseDown(interaction: MouseDownInteraction) {
     if (rectContainsPoint(interaction.payload.location, this.frame)) {
-      this._isPanning = true;
+      const frameHeight = this.frame.size.height;
+      const contentHeight = this._contentView.desiredSize().height;
+      // Don't claim drag operations if the content is not tall enough to be scrollable.
+      // This would block any outer scroll views from working.
+      if (frameHeight < contentHeight) {
+        this._isPanning = true;
+      }
     }
   }
 
@@ -179,6 +205,7 @@ export class VerticalScrollView extends View {
       containerLength: this.frame.size.height,
     });
     this._setScrollState(newState);
+    return true;
   }
 
   _handleMouseUp(interaction: MouseUpInteraction) {
@@ -187,31 +214,27 @@ export class VerticalScrollView extends View {
     }
   }
 
-  _handleWheelShift(interaction: WheelWithShiftInteraction) {
+  _handleWheelShift(interaction: WheelWithShiftInteraction): boolean {
     const {
       location,
       delta: {deltaX, deltaY},
     } = interaction.payload;
+
     if (!rectContainsPoint(location, this.frame)) {
-      return; // Not scrolling on view
+      return false; // Not scrolling on view
     }
 
     const absDeltaX = Math.abs(deltaX);
     const absDeltaY = Math.abs(deltaY);
     if (absDeltaX > absDeltaY) {
-      return; // Scrolling horizontally
+      return false; // Scrolling horizontally
     }
 
     if (absDeltaY < MOVE_WHEEL_DELTA_THRESHOLD) {
-      return;
+      return false; // Movement was too small and should be ignored.
     }
 
-    const newState = translateState({
-      state: this._scrollState,
-      delta: -deltaY,
-      containerLength: this.frame.size.height,
-    });
-    this._setScrollState(newState);
+    return this.scrollBy(deltaY);
   }
 
   _restoreMutableViewState() {
@@ -231,10 +254,7 @@ export class VerticalScrollView extends View {
     this.setNeedsDisplay();
   }
 
-  /**
-   * @private
-   */
-  _setScrollState(proposedState: ScrollState) {
+  _setScrollState(proposedState: ScrollState): boolean {
     const height = this._contentView.frame.size.height;
     const clampedState = clampState({
       state: proposedState,
@@ -247,6 +267,14 @@ export class VerticalScrollView extends View {
       this._scrollState.length = clampedState.length;
 
       this.setNeedsDisplay();
+
+      if (this._onChangeCallback !== null) {
+        this._onChangeCallback(clampedState, this.frame.size.height);
+      }
+
+      return true;
     }
+
+    return false;
   }
 }

packages/react-devtools-scheduling-profiler/src/view-base/VerticalScrollView.js

@@ -29,6 +29,8 @@ import {noopLayout, viewsToLayout, collapseLayoutIntoViews} from './layouter';
  * subclasses.
  */
 export class View {
+  _backgroundColor: string | null;
+
   currentCursor: string | null = null;
 
   surface: Surface;
@@ -70,7 +72,9 @@ export class View {
     frame: Rect,
     layouter: Layouter = noopLayout,
     visibleArea: Rect = frame,
+    backgroundColor?: string | null = null,
   ) {
+    this._backgroundColor = backgroundColor || null;
     this.surface = surface;
     this.frame = frame;
     this._layouter = layouter;
@@ -246,6 +250,20 @@ export class View {
         subview.displayIfNeeded(context, viewRefs);
       }
     });
+
+    const backgroundColor = this._backgroundColor;
+    if (backgroundColor !== null) {
+      const desiredSize = this.desiredSize();
+      if (visibleArea.size.height > desiredSize.height) {
+        context.fillStyle = backgroundColor;
+        context.fillRect(
+          visibleArea.origin.x,
+          visibleArea.origin.y + desiredSize.height,
+          visibleArea.size.width,
+          visibleArea.size.height - desiredSize.height,
+        );
+      }
+    }
   }
 
   /**
@@ -255,7 +273,7 @@ export class View {
    *
    * NOTE: Do not call directly! Use `handleInteractionAndPropagateToSubviews`
    */
-  handleInteraction(interaction: Interaction, viewRefs: ViewRefs) {}
+  handleInteraction(interaction: Interaction, viewRefs: ViewRefs): ?boolean {}
 
   /**
    * Handle an `interaction` and propagates it to all of this view's
@@ -270,19 +288,39 @@ export class View {
   handleInteractionAndPropagateToSubviews(
     interaction: Interaction,
     viewRefs: ViewRefs,
-  ) {
+  ): boolean {
     const {subviews, visibleArea} = this;
 
     if (visibleArea.size.height === 0) {
-      return;
+      return false;
     }
 
-    this.handleInteraction(interaction, viewRefs);
-
-    subviews.forEach(subview => {
+    // Pass the interaction to subviews first,
+    // so they have the opportunity to claim it before it bubbles.
+    //
+    // Views are painted first to last,
+    // so they should process interactions last to first,
+    // so views in front (on top) can claim the interaction first.
+    for (let i = subviews.length - 1; i >= 0; i--) {
+      const subview = subviews[i];
       if (rectIntersectsRect(visibleArea, subview.visibleArea)) {
-        subview.handleInteractionAndPropagateToSubviews(interaction, viewRefs);
+        const didSubviewHandle =
+          subview.handleInteractionAndPropagateToSubviews(
+            interaction,
+            viewRefs,
+          ) === true;
+        if (didSubviewHandle) {
+          return true;
+        }
       }
-    });
+    }
+
+    const didSelfHandle =
+      this.handleInteraction(interaction, viewRefs) === true;
+    if (didSelfHandle) {
+      return true;
+    }
+
+    return false;
   }
 }

packages/react-devtools-scheduling-profiler/src/view-base/View.js

@@ -9,10 +9,11 @@
 
 export * from './BackgroundColorView';
 export * from './HorizontalPanAndZoomView';
-export * from './ResizableView';
 export * from './Surface';
 export * from './VerticalScrollView';
 export * from './View';
 export * from './geometry';
 export * from './layouter';
+export * from './resizable';
 export * from './useCanvasInteraction';
+export * from './vertical-scroll-overflow';

packages/react-devtools-scheduling-profiler/src/view-base/index.js

@@ -206,50 +206,6 @@ export const atLeastContainerHeightLayout: Layouter = (
   }));
 };
 
-/**
- * Forces last view to take up the space below the second-last view.
- * Intended to be used with a vertical stack layout.
- */
-export const lastViewTakesUpRemainingSpaceLayout: Layouter = (
-  layout,
-  containerFrame,
-) => {
-  if (layout.length === 0) {
-    // Nothing to do
-    return layout;
-  }
-
-  if (layout.length === 1) {
-    // No second-last view; the view should just take up the container height
-    return containerHeightLayout(layout, containerFrame);
-  }
-
-  const layoutInfoToPassThrough = layout.slice(0, layout.length - 1);
-  const secondLastLayoutInfo =
-    layoutInfoToPassThrough[layoutInfoToPassThrough.length - 1];
-
-  const remainingHeight =
-    containerFrame.size.height -
-    secondLastLayoutInfo.frame.origin.y -
-    secondLastLayoutInfo.frame.size.height;
-  const height = Math.max(remainingHeight, 0); // Prevent negative heights
-
-  const lastLayoutInfo = layout[layout.length - 1];
-  return [
-    ...layoutInfoToPassThrough,
-    {
-      ...lastLayoutInfo,
-      frame: {
-        origin: lastLayoutInfo.frame.origin,
-        size: {
-          width: lastLayoutInfo.frame.size.width,
-          height,
-        },
-      },
-    },
-  ];
-};
-
 /**
  * Create a layouter that applies each layouter in `layouters` in sequence.
  */