Browse Source

feat(profiling): pin columns (#35612)

* feat(calltree): add collapse recursion checkbox

* feat(calltree): move the resizable logic to hook

* feat(calltree): add skipped field to tree

* feat(calltree): add expanded history

* feat(calltree): add expanded history

* ref(scroll): improve scroll performance

wip

* feat(profiling): add ghost row for hover/selected state

* feat(profiling): simulate click and mouseenter when outside of width

* test(usevirtualizedtree): mock remove listener

* ref(useVirtualizedTree): memo rendering

* ref(useVirtualizedTree): fire another raf in case promise never resolved

* feat(profiling): pin weight columns

* ref(virtualizedTree): use mode for ghost rows and minor review feedback
Jonas 2 years ago
parent
commit
2c4769853b

+ 15 - 0
static/app/components/profiling/FrameStack/frameStack.tsx

@@ -240,6 +240,7 @@ const FrameTabs = styled('ul')`
 
 const FRAME_WEIGHT_CELL_WIDTH_PX = 164;
 export const FrameCallersTableCell = styled('div')<{
+  isSelected?: boolean;
   noPadding?: boolean;
   textAlign?: React.CSSProperties['textAlign'];
 }>`
@@ -250,6 +251,20 @@ export const FrameCallersTableCell = styled('div')<{
   padding: 0 ${p => (p.noPadding ? 0 : space(1))} 0 0;
   text-align: ${p => p.textAlign ?? 'initial'};
 
+  &:first-child,
+  &:nth-child(2) {
+    position: sticky;
+    z-index: 1;
+    background-color: ${p => (p.isSelected ? p.theme.blue300 : p.theme.background)};
+  }
+
+  &:first-child {
+    left: 0;
+  }
+  &:nth-child(2) {
+    left: ${FRAME_WEIGHT_CELL_WIDTH_PX}px;
+  }
+
   &:not(:last-child) {
     border-right: 1px solid ${p => p.theme.border};
   }

+ 15 - 13
static/app/components/profiling/FrameStack/frameStackTable.tsx

@@ -13,6 +13,7 @@ import {
 } from 'sentry/utils/profiling/hooks/useVirtualizedTree/useVirtualizedTree';
 import {VirtualizedTreeNode} from 'sentry/utils/profiling/hooks/useVirtualizedTree/VirtualizedTreeNode';
 import {FlamegraphRenderer} from 'sentry/utils/profiling/renderers/flamegraphRenderer';
+import theme from 'sentry/utils/theme';
 
 import {FrameCallersTableCell} from './frameStack';
 import {FrameStackContextMenu} from './frameStackContextMenu';
@@ -214,7 +215,7 @@ export function FrameStackTable({
           onZoomIntoNodeClick={handleZoomIntoNodeClick}
           contextMenu={contextMenu}
         />
-        <div style={{position: 'relative', height: '100%'}}>
+        <div style={{position: 'relative', height: '100%', background: theme.white}}>
           <div ref={clickedGhostRowRef} />
           <div ref={hoveredGhostRowRef} />
           <div
@@ -229,19 +230,11 @@ export function FrameStackTable({
               so that borders on columns are shown across the entire table and not just the rows.
               This is useful when number of rows does not fill up the entire table height.
              */}
-              <div
-                style={{
-                  display: 'flex',
-                  width: '100%',
-                  pointerEvents: 'none',
-                  position: 'absolute',
-                  height: '100%',
-                }}
-              >
+              <GhostRowContainer>
                 <FrameCallersTableCell />
                 <FrameCallersTableCell />
-                <FrameCallersTableCell />
-              </div>
+                <FrameCallersTableCell style={{width: '100%'}} />
+              </GhostRowContainer>
             </div>
           </div>
         </div>
@@ -250,6 +243,15 @@ export function FrameStackTable({
   );
 }
 
+const GhostRowContainer = styled('div')`
+  display: flex;
+  width: 100%;
+  pointer-events: none;
+  position: absolute;
+  height: 100%;
+  z-index: -1;
+`;
+
 const TableHeaderButton = styled('button')`
   display: flex;
   width: 100%;
@@ -310,7 +312,7 @@ const FrameCallersTableHeader = styled('div')`
   > div {
     position: relative;
     border-bottom: 1px solid ${p => p.theme.border};
-    background-color: ${p => p.theme.surface400};
+    background-color: ${p => p.theme.background};
     white-space: nowrap;
 
     &:last-child {

+ 15 - 7
static/app/components/profiling/FrameStack/frameStackTableRow.tsx

@@ -50,6 +50,7 @@ export const FrameStackTableRow = forwardRef<HTMLDivElement, FrameStackTableRowP
     },
     ref
   ) => {
+    const isSelected = tabIndex === 0;
     const colorString = useMemo(() => {
       return formatColorForFrame(node.node, flamegraphRenderer);
     }, [node, flamegraphRenderer]);
@@ -68,34 +69,34 @@ export const FrameStackTableRow = forwardRef<HTMLDivElement, FrameStackTableRowP
         style={style}
         onContextMenu={onContextMenu}
         tabIndex={tabIndex}
-        isSelected={tabIndex === 0}
+        isSelected={isSelected}
         onKeyDown={onKeyDown}
         onClick={onClick}
         onMouseEnter={onMouseEnter}
       >
-        <FrameCallersTableCell textAlign="right">
+        <FrameCallersTableCell isSelected={isSelected} textAlign="right">
           {flamegraphRenderer.flamegraph.formatter(node.node.node.selfWeight)}
           <Weight
-            isSelected={tabIndex === 0}
+            isSelected={isSelected}
             weight={computeRelativeWeight(
               referenceNode.node.totalWeight,
               node.node.node.selfWeight
             )}
           />
         </FrameCallersTableCell>
-        <FrameCallersTableCell noPadding textAlign="right">
+        <FrameCallersTableCell isSelected={isSelected} noPadding textAlign="right">
           <FrameWeightTypeContainer>
             <FrameWeightContainer>
               {flamegraphRenderer.flamegraph.formatter(node.node.node.totalWeight)}
               <Weight
-                isSelected={tabIndex === 0}
+                isSelected={isSelected}
                 weight={computeRelativeWeight(
                   referenceNode.node.totalWeight,
                   node.node.node.totalWeight
                 )}
               />
             </FrameWeightContainer>
-            <FrameTypeIndicator isSelected={tabIndex === 0}>
+            <FrameTypeIndicator isSelected={isSelected}>
               {node.node.node.frame.is_application ? (
                 <IconUser size="xs" />
               ) : (
@@ -180,12 +181,19 @@ const BackgroundWeightBar = styled('div')`
 
 const FrameCallersRow = styled('div')<{isSelected: boolean}>`
   display: flex;
-  width: 100%;
+  width: calc(100% + 400px);
   color: ${p => (p.isSelected ? p.theme.white : 'inherit')};
 
   &:focus {
     outline: none;
   }
+
+  &[data-hovered='true']:not([tabindex='0']) {
+    > div:first-child,
+    > div:nth-child(2) {
+      background-color: #edf2fc !important;
+    }
+  }
 `;
 
 const FrameNameContainer = styled('div')`

+ 35 - 8
static/app/utils/profiling/hooks/useVirtualizedTree/useVirtualizedTree.tsx

@@ -190,6 +190,8 @@ export function useVirtualizedTree<T extends TreeLike>(
   const clickedGhostRowRef = useRef<HTMLDivElement | null>(null);
   const hoveredGhostRowRef = useRef<HTMLDivElement | null>(null);
 
+  const previousHoveredRow = useRef<number | null>(null);
+
   const [state, dispatch] = useReducer(VirtualizedTreeStateReducer, {
     scrollTop: 0,
     roots: props.roots,
@@ -201,7 +203,6 @@ export function useVirtualizedTree<T extends TreeLike>(
   // Keep a ref to latest state to avoid re-rendering
   const latestStateRef = useRef<typeof state>(state);
   latestStateRef.current = state;
-
   const [tree, setTree] = useState(() => {
     const initialTree = VirtualizedTree.fromRoots(props.roots, props.skipFunction);
 
@@ -212,6 +213,14 @@ export function useVirtualizedTree<T extends TreeLike>(
     return initialTree;
   });
 
+  const cleanupAllHoveredRows = useCallback(() => {
+    for (const row of latestItemsRef.current) {
+      if (row.ref && row.ref.dataset.hovered) {
+        delete row.ref.dataset.hovered;
+      }
+    }
+  }, []);
+
   const expandedHistory = useRef<Set<T>>(new Set());
   useEffectAfterFirstRender(() => {
     const expandedNodes = tree.getAllExpandedNodes(expandedHistory.current);
@@ -245,11 +254,13 @@ export function useVirtualizedTree<T extends TreeLike>(
     } else {
       hideGhostRow({ref: clickedGhostRowRef});
     }
+
+    cleanupAllHoveredRows();
     hideGhostRow({ref: hoveredGhostRowRef});
 
     dispatch({type: 'set tab index key', payload: tabIndex});
     setTree(newTree);
-  }, [props.roots, props.skipFunction]);
+  }, [props.roots, props.skipFunction, cleanupAllHoveredRows]);
 
   const {rowHeight, renderRow} = props;
   const items = useMemo(() => {
@@ -341,6 +352,8 @@ export function useVirtualizedTree<T extends TreeLike>(
           rowHeight: props.rowHeight,
         });
       }
+
+      cleanupAllHoveredRows();
       hideGhostRow({
         ref: hoveredGhostRowRef,
       });
@@ -353,7 +366,7 @@ export function useVirtualizedTree<T extends TreeLike>(
     return () => {
       scrollContainer.removeEventListener('scroll', handleScroll);
     };
-  }, [props.scrollContainer, props.rowHeight]);
+  }, [props.scrollContainer, props.rowHeight, cleanupAllHoveredRows]);
 
   useEffect(() => {
     const scrollContainer = props.scrollContainer;
@@ -404,6 +417,12 @@ export function useVirtualizedTree<T extends TreeLike>(
         (latestStateRef.current.scrollTop + evt.clientY - rect.top) / props.rowHeight
       );
 
+      cleanupAllHoveredRows();
+      const element = latestItemsRef.current.find(item => item.key === index);
+      if (element?.ref) {
+        element.ref.dataset.hovered = 'true';
+      }
+
       // If a node exists at the index, select it
       if (tree.flattened[index] && index !== latestStateRef.current.tabIndexKey) {
         updateGhostRow({
@@ -423,7 +442,7 @@ export function useVirtualizedTree<T extends TreeLike>(
       scrollContainer.removeEventListener('click', handleClick);
       scrollContainer.removeEventListener('mousemove', handleMouseMove);
     };
-  }, [props.rowHeight, props.scrollContainer, tree.flattened]);
+  }, [props.rowHeight, props.scrollContainer, tree.flattened, cleanupAllHoveredRows]);
 
   // When mouseleave is triggered on the contianer,
   // we need to hide the ghost row to avoid an orphaned row
@@ -434,6 +453,7 @@ export function useVirtualizedTree<T extends TreeLike>(
     }
 
     function onMouseLeave() {
+      cleanupAllHoveredRows();
       hideGhostRow({
         ref: hoveredGhostRowRef,
       });
@@ -445,7 +465,7 @@ export function useVirtualizedTree<T extends TreeLike>(
     return () => {
       container.removeEventListener('mouseleave', onMouseLeave);
     };
-  }, [props.scrollContainer]);
+  }, [cleanupAllHoveredRows, props.scrollContainer]);
 
   // When a node is expanded, the underlying tree is recomputed (the flattened tree is updated)
   // We copy the properties of the old tree by creating a new instance of VirtualizedTree
@@ -609,6 +629,12 @@ export function useVirtualizedTree<T extends TreeLike>(
   const handleRowMouseEnter = useCallback(
     (key: number) => {
       return (_evt: React.MouseEvent<HTMLElement>) => {
+        if (previousHoveredRow.current !== key) {
+          cleanupAllHoveredRows();
+
+          (_evt.currentTarget as HTMLElement).dataset.hovered = 'true';
+          previousHoveredRow.current = key;
+        }
         updateGhostRow({
           ref: hoveredGhostRowRef,
           tabIndexKey: key,
@@ -618,7 +644,7 @@ export function useVirtualizedTree<T extends TreeLike>(
         });
       };
     },
-    [state.scrollTop, props.rowHeight]
+    [state.scrollTop, props.rowHeight, cleanupAllHoveredRows]
   );
 
   // Register a resize observer for when the scroll container is resized.
@@ -635,6 +661,7 @@ export function useVirtualizedTree<T extends TreeLike>(
           type: 'set scroll height',
           payload: elements[0]?.contentRect?.height ?? 0,
         });
+        cleanupAllHoveredRows();
         hideGhostRow({
           ref: hoveredGhostRowRef,
         });
@@ -649,7 +676,7 @@ export function useVirtualizedTree<T extends TreeLike>(
       }
       resizeObserver.disconnect();
     };
-  }, [props.scrollContainer]);
+  }, [props.scrollContainer, cleanupAllHoveredRows]);
 
   // Basic required styles for the scroll container
   const scrollContainerStyles: React.CSSProperties = useMemo(() => {
@@ -665,7 +692,7 @@ export function useVirtualizedTree<T extends TreeLike>(
   // scrollbar is sized according to the number of items in the list.
   const containerStyles: React.CSSProperties = useMemo(() => {
     const height = tree.flattened.length * props.rowHeight;
-    return {height, maxHeight: height, overflow: 'hidden'};
+    return {height, maxHeight: height};
   }, [tree.flattened.length, props.rowHeight]);
 
   const renderedItems: React.ReactNode[] = useMemo(() => {