From b450dbe47f404d6d84aaf7ad9a69434e807515c0 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 27 Mar 2025 19:00:11 -0400 Subject: [PATCH 1/3] useResizeableDrawer: remove size state storage --- .../interfaces/breadcrumbs/breadcrumbs.tsx | 16 +++----- .../differentialFlamegraphLayout.tsx | 6 +-- .../flamegraphDrawer/profileDetails.tsx | 6 +-- .../profiling/flamegraph/flamegraphLayout.tsx | 6 +-- .../virtualizedGrid/useDetailsSplit.tsx | 9 +++-- static/app/components/splitPanel.tsx | 24 ++++++++---- static/app/utils/useResizableDrawer.tsx | 38 ++++--------------- 7 files changed, 44 insertions(+), 61 deletions(-) diff --git a/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx b/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx index 3bd264cb98a8f0..c0556cceaefeb5 100644 --- a/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx +++ b/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx @@ -1,5 +1,5 @@ import type React from 'react'; -import {Fragment, useCallback, useEffect, useMemo, useRef} from 'react'; +import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react'; import type {ListProps} from 'react-virtualized'; import {AutoSizer, CellMeasurer, CellMeasurerCache, List} from 'react-virtualized'; import type {ListRowRenderer} from 'react-virtualized/dist/es/List'; @@ -29,8 +29,6 @@ import {Breadcrumb} from './breadcrumb'; const PANEL_MIN_HEIGHT = 200; export const PANEL_INITIAL_HEIGHT = 400; -const noop = () => void 0; - const cache = new CellMeasurerCache({ fixedWidth: true, defaultHeight: 38, @@ -158,14 +156,10 @@ function Breadcrumbs({ } }, []); - const { - size: containerSize, - isHeld, - onMouseDown, - onDoubleClick, - } = useResizableDrawer({ + const [size, setSize] = useState(PANEL_INITIAL_HEIGHT); + const {isHeld, onMouseDown, onDoubleClick} = useResizableDrawer({ direction: 'down', - onResize: noop, + onResize: s => setSize(s), initialSize: PANEL_INITIAL_HEIGHT, min: PANEL_MIN_HEIGHT, }); @@ -208,7 +202,7 @@ function Breadcrumbs({ { + const onResize = (newSize: number) => { if (!flamegraphDrawerRef.current) { return; } if (isSidebarLayout) { - flamegraphDrawerRef.current.style.width = `${maybeOldSize ?? newSize}px`; + flamegraphDrawerRef.current.style.width = `${newSize}px`; flamegraphDrawerRef.current.style.height = `100%`; } else { - flamegraphDrawerRef.current.style.height = `${maybeOldSize ?? newSize}px`; + flamegraphDrawerRef.current.style.height = `${newSize}px`; flamegraphDrawerRef.current.style.width = `100%`; } }; diff --git a/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx b/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx index 16e6fa7dd85413..cc32fa43e7c3aa 100644 --- a/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx +++ b/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx @@ -91,14 +91,14 @@ export function ProfileDetails(props: ProfileDetailsProps) { // Only used when in sidebar layout const initialSize = isSidebarLayout ? 260 : 0; - const onResize = (newSize: number, maybeOldSize?: number) => { + const onResize = (delta: number) => { if (!detailsBarRef.current) { return; } if (isSidebarLayout) { - detailsBarRef.current.style.width = `100%`; - detailsBarRef.current.style.height = `${maybeOldSize ?? newSize}px`; + detailsBarRef.current.style.width = `${initialSize + delta}px`; + detailsBarRef.current.style.height = `100%`; } else { detailsBarRef.current.style.height = ''; detailsBarRef.current.style.width = ''; diff --git a/static/app/components/profiling/flamegraph/flamegraphLayout.tsx b/static/app/components/profiling/flamegraph/flamegraphLayout.tsx index 1ccc021fafb4a2..745f8b7fd063f1 100644 --- a/static/app/components/profiling/flamegraph/flamegraphLayout.tsx +++ b/static/app/components/profiling/flamegraph/flamegraphLayout.tsx @@ -50,16 +50,16 @@ export function FlamegraphLayout(props: FlamegraphLayoutProps) { ? MIN_FLAMEGRAPH_DRAWER_DIMENSIONS[0] : MIN_FLAMEGRAPH_DRAWER_DIMENSIONS[1]; - const onResize = (newSize: number, maybeOldSize: number | undefined) => { + const onResize = (newSize: number) => { if (!flamegraphDrawerRef.current) { return; } if (isSidebarLayout) { - flamegraphDrawerRef.current.style.width = `${maybeOldSize ?? newSize}px`; + flamegraphDrawerRef.current.style.width = `${newSize}px`; flamegraphDrawerRef.current.style.height = `100%`; } else { - flamegraphDrawerRef.current.style.height = `${maybeOldSize ?? newSize}px`; + flamegraphDrawerRef.current.style.height = `${newSize}px`; flamegraphDrawerRef.current.style.width = `100%`; } }; diff --git a/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx b/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx index e28cef3a7b8fa0..900b8c83f957d9 100644 --- a/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx +++ b/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx @@ -1,5 +1,5 @@ import type {RefObject} from 'react'; -import {useCallback} from 'react'; +import {useCallback, useState} from 'react'; import {useResizableDrawer} from 'sentry/utils/useResizableDrawer'; import useUrlParams from 'sentry/utils/useUrlParams'; @@ -52,12 +52,15 @@ export default function useDetailsSplit({ // `initialSize` cannot depend on containerRef because the ref starts as // `undefined` which then gets set into the hook and doesn't update. const initialSize = Math.max(150, window.innerHeight * 0.4); + const [containerSize, setContainerSize] = useState(initialSize); - const {size: containerSize, ...resizableDrawerProps} = useResizableDrawer({ + const resizableDrawerProps = useResizableDrawer({ direction: 'up', initialSize, min: 0, - onResize: () => {}, + onResize: (newSize: number) => { + setContainerSize(newSize); + }, }); const maxContainerHeight = diff --git a/static/app/components/splitPanel.tsx b/static/app/components/splitPanel.tsx index 84afcc910e8686..8512b95eb9be7f 100644 --- a/static/app/components/splitPanel.tsx +++ b/static/app/components/splitPanel.tsx @@ -1,4 +1,4 @@ -import {createContext, useCallback, useMemo} from 'react'; +import {createContext, useCallback, useMemo, useState} from 'react'; import styled from '@emotion/styled'; import {IconGrabbable} from 'sentry/icons'; @@ -110,17 +110,25 @@ function SplitPanel(props: SplitPanelProps) { const min = isLeftRight ? props.left.min : props.top.min; const max = isLeftRight ? props.left.max : props.top.max; + const [containerSize, setContainerSize] = useState(() => { + const storedSize = sizeStorageKey + ? parseInt(localStorage.getItem(sizeStorageKey) ?? '', 10) + : undefined; + return storedSize ?? initialSize; + }); + const { isHeld, onDoubleClick, onMouseDown: onDragStart, - size: containerSize, - setSize, } = useResizableDrawer({ direction: isLeftRight ? 'left' : 'down', initialSize, min, - onResize: onResize ?? (() => {}), + onResize: (size: number) => { + setContainerSize(size); + onResize?.(size); + }, sizeStorageKey, }); @@ -141,11 +149,11 @@ function SplitPanel(props: SplitPanelProps) { () => ({ isMaximized, isMinimized, - maximiseSize: () => setSize(max), - minimiseSize: () => setSize(min), - resetSize: () => setSize(initialSize), + maximiseSize: () => setContainerSize(max), + minimiseSize: () => setContainerSize(min), + resetSize: () => setContainerSize(initialSize), }), - [isMaximized, isMinimized, setSize, max, min, initialSize] + [isMaximized, isMinimized, setContainerSize, max, min, initialSize] ); if (isLeftRight) { diff --git a/static/app/utils/useResizableDrawer.tsx b/static/app/utils/useResizableDrawer.tsx index 75045ac0b72482..361bac6d07b6fc 100644 --- a/static/app/utils/useResizableDrawer.tsx +++ b/static/app/utils/useResizableDrawer.tsx @@ -16,11 +16,7 @@ export interface UseResizableDrawerOptions { /** * Triggered while dragging */ - onResize: ( - newSize: number, - maybeOldSize: number | undefined, - userEvent: boolean - ) => void; + onResize: (size: number, userEvent: boolean) => void; /** * The local storage key used to persist the size of the container */ @@ -51,32 +47,16 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { * Call this function to manually set the size of the drawer. */ setSize: (newSize: number, userEvent?: boolean) => void; - /** - * The resulting size of the container axis. Updated while dragging. - * - * NOTE: Be careful using this as this as react state updates are not - * synchronous, you may want to update the element size using onResize instead - */ - size: number; } { const rafIdRef = useRef(null); const currentMouseVectorRaf = useRef<[number, number] | null>(null); - const [size, setSize] = useState(() => { - const storedSize = options.sizeStorageKey - ? parseInt(localStorage.getItem(options.sizeStorageKey) ?? '', 10) - : undefined; - - return storedSize || options.initialSize; - }); const [isHeld, setIsHeld] = useState(false); + const sizeRef = useRef(options.initialSize); const updateSize = useCallback( (newSize: number, userEvent = false) => { - setSize(newSize); - options.onResize(newSize, undefined, userEvent); - if (options.sizeStorageKey) { - localStorage.setItem(options.sizeStorageKey, newSize.toString()); - } + sizeRef.current = newSize; + options.onResize(newSize, userEvent); }, [options] ); @@ -85,14 +65,12 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { // any potentional values set by CSS will be overriden. If no initialDimensions are provided, // invoke the onResize callback with the previously stored dimensions. useLayoutEffect(() => { - options.onResize(options.initialSize ?? 0, size, false); - setSize(options.initialSize ?? 0); + sizeRef.current = options.initialSize; + options.onResize(options.initialSize ?? 0, false); + // setSize(options.initialSize ?? 0); // eslint-disable-next-line react-hooks/exhaustive-deps }, [options.direction]); - const sizeRef = useRef(size); - sizeRef.current = size; - const onMouseMove = useCallback( (event: MouseEvent) => { event.stopPropagation(); @@ -169,5 +147,5 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { }; }); - return {size, isHeld, onMouseDown, onDoubleClick, setSize: updateSize}; + return {isHeld, onMouseDown, onDoubleClick, setSize: updateSize}; } From 1d798960436b215811186a964d63a03e76992a26 Mon Sep 17 00:00:00 2001 From: JonasBa Date: Thu, 27 Mar 2025 19:01:19 -0400 Subject: [PATCH 2/3] useResizeableDrawer: remove size state storage --- static/app/utils/useResizableDrawer.tsx | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/static/app/utils/useResizableDrawer.tsx b/static/app/utils/useResizableDrawer.tsx index 361bac6d07b6fc..9be2f561d96f3c 100644 --- a/static/app/utils/useResizableDrawer.tsx +++ b/static/app/utils/useResizableDrawer.tsx @@ -67,7 +67,6 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { useLayoutEffect(() => { sizeRef.current = options.initialSize; options.onResize(options.initialSize ?? 0, false); - // setSize(options.initialSize ?? 0); // eslint-disable-next-line react-hooks/exhaustive-deps }, [options.direction]); @@ -116,6 +115,11 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { ); const onMouseUp = useCallback(() => { + if (rafIdRef.current !== null) { + window.cancelAnimationFrame(rafIdRef.current); + rafIdRef.current = null; + } + document.body.style.pointerEvents = ''; document.body.style.userSelect = ''; document.documentElement.style.cursor = ''; @@ -139,13 +143,5 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { updateSize(options.initialSize, true); }, [updateSize, options.initialSize]); - useLayoutEffect(() => { - return () => { - if (rafIdRef.current !== null) { - window.cancelAnimationFrame(rafIdRef.current); - } - }; - }); - return {isHeld, onMouseDown, onDoubleClick, setSize: updateSize}; } From 82380c1b18028838de90a473eb0e799790c58312 Mon Sep 17 00:00:00 2001 From: Jonas Date: Tue, 8 Apr 2025 10:39:44 -0400 Subject: [PATCH 3/3] useResizeableDrawer: remove setSize (#88172) --- .../interfaces/breadcrumbs/breadcrumbs.tsx | 6 +++++- .../differentialFlamegraphLayout.tsx | 6 +++++- .../flamegraphDrawer/profileDetails.tsx | 6 +++++- .../profiling/flamegraph/flamegraphLayout.tsx | 6 +++++- .../virtualizedGrid/detailsSplitDivider.tsx | 1 + .../virtualizedGrid/useDetailsSplit.tsx | 10 +++++++++- static/app/components/splitPanel.tsx | 19 ++++++++++--------- static/app/utils/useResizableDrawer.tsx | 14 +------------- .../replays/detail/network/details/index.tsx | 1 + 9 files changed, 42 insertions(+), 27 deletions(-) diff --git a/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx b/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx index c0556cceaefeb5..e2e7bca55d635f 100644 --- a/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx +++ b/static/app/components/events/interfaces/breadcrumbs/breadcrumbs.tsx @@ -157,13 +157,17 @@ function Breadcrumbs({ }, []); const [size, setSize] = useState(PANEL_INITIAL_HEIGHT); - const {isHeld, onMouseDown, onDoubleClick} = useResizableDrawer({ + const {isHeld, onMouseDown} = useResizableDrawer({ direction: 'down', onResize: s => setSize(s), initialSize: PANEL_INITIAL_HEIGHT, min: PANEL_MIN_HEIGHT, }); + const onDoubleClick = useCallback(() => { + setSize(PANEL_INITIAL_HEIGHT); + }, []); + const panelHeaders: PanelTableProps['headers'] = useMemo(() => { return [ t('Type'), diff --git a/static/app/components/profiling/flamegraph/differentialFlamegraphLayout.tsx b/static/app/components/profiling/flamegraph/differentialFlamegraphLayout.tsx index 53f39c0ad07f54..8b439395d83bb3 100644 --- a/static/app/components/profiling/flamegraph/differentialFlamegraphLayout.tsx +++ b/static/app/components/profiling/flamegraph/differentialFlamegraphLayout.tsx @@ -66,7 +66,11 @@ export function DifferentialFlamegraphLayout(props: DifferentialFlamegraphLayout }; }, [layout]); - const {onMouseDown, onDoubleClick} = useResizableDrawer(resizableOptions); + const {onMouseDown} = useResizableDrawer(resizableOptions); + + const onDoubleClick = useCallback(() => { + resizableOptions.onResize?.(resizableOptions.initialSize, true); + }, [resizableOptions]); const onOpenMinimap = useCallback( () => diff --git a/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx b/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx index cc32fa43e7c3aa..e83a35c4d71320 100644 --- a/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx +++ b/static/app/components/profiling/flamegraph/flamegraphDrawer/profileDetails.tsx @@ -113,7 +113,11 @@ export function ProfileDetails(props: ProfileDetailsProps) { }; }, [flamegraphPreferences.layout]); - const {onMouseDown, onDoubleClick} = useResizableDrawer(resizableOptions); + const {onMouseDown} = useResizableDrawer(resizableOptions); + + const onDoubleClick = useCallback(() => { + resizableOptions.onResize?.(resizableOptions.initialSize, true); + }, [resizableOptions]); return ( diff --git a/static/app/components/profiling/flamegraph/flamegraphLayout.tsx b/static/app/components/profiling/flamegraph/flamegraphLayout.tsx index 745f8b7fd063f1..78f02cd4e758b1 100644 --- a/static/app/components/profiling/flamegraph/flamegraphLayout.tsx +++ b/static/app/components/profiling/flamegraph/flamegraphLayout.tsx @@ -73,7 +73,11 @@ export function FlamegraphLayout(props: FlamegraphLayoutProps) { }; }, [layout]); - const {onMouseDown, onDoubleClick} = useResizableDrawer(resizableOptions); + const {onMouseDown} = useResizableDrawer(resizableOptions); + + const onDoubleClick = useCallback(() => { + resizableOptions.onResize?.(resizableOptions.initialSize, true); + }, [resizableOptions]); const onOpenMinimap = useCallback( () => diff --git a/static/app/components/replays/virtualizedGrid/detailsSplitDivider.tsx b/static/app/components/replays/virtualizedGrid/detailsSplitDivider.tsx index 162e8856395a2d..0edb95ac00a355 100644 --- a/static/app/components/replays/virtualizedGrid/detailsSplitDivider.tsx +++ b/static/app/components/replays/virtualizedGrid/detailsSplitDivider.tsx @@ -11,6 +11,7 @@ import SplitDivider from 'sentry/views/replays/detail/layout/splitDivider'; interface Props extends Omit, 'size' | 'setSize'> { onClose: () => void; + onDoubleClick: () => void; children?: ReactNode; } diff --git a/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx b/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx index 900b8c83f957d9..9fac1bfd22597f 100644 --- a/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx +++ b/static/app/components/replays/virtualizedGrid/useDetailsSplit.tsx @@ -63,6 +63,10 @@ export default function useDetailsSplit({ }, }); + const onDoubleClick = useCallback(() => { + setContainerSize(initialSize); + }, [initialSize]); + const maxContainerHeight = (containerRef.current?.clientHeight || window.innerHeight) - handleHeight; const splitSize = @@ -71,7 +75,11 @@ export default function useDetailsSplit({ return { onClickCell, onCloseDetailsSplit, - resizableDrawerProps, + onDoubleClick, + resizableDrawerProps: { + ...resizableDrawerProps, + onDoubleClick, + }, selectedIndex: getDetailIndex(), splitSize, }; diff --git a/static/app/components/splitPanel.tsx b/static/app/components/splitPanel.tsx index 8512b95eb9be7f..d3b86aa7b2288e 100644 --- a/static/app/components/splitPanel.tsx +++ b/static/app/components/splitPanel.tsx @@ -106,22 +106,19 @@ function SplitPanel(props: SplitPanelProps) { sizeStorageKey, } = props; const isLeftRight = 'left' in props; - const initialSize = isLeftRight ? props.left.default : props.top.default; const min = isLeftRight ? props.left.min : props.top.min; const max = isLeftRight ? props.left.max : props.top.max; - const [containerSize, setContainerSize] = useState(() => { + const initialSize = useMemo(() => { const storedSize = sizeStorageKey ? parseInt(localStorage.getItem(sizeStorageKey) ?? '', 10) : undefined; - return storedSize ?? initialSize; - }); + return storedSize ?? (isLeftRight ? props.left.default : props.top.default); + }, [sizeStorageKey, props, isLeftRight]); - const { - isHeld, - onDoubleClick, - onMouseDown: onDragStart, - } = useResizableDrawer({ + const [containerSize, setContainerSize] = useState(initialSize); + + const {isHeld, onMouseDown: onDragStart} = useResizableDrawer({ direction: isLeftRight ? 'left' : 'down', initialSize, min, @@ -156,6 +153,10 @@ function SplitPanel(props: SplitPanelProps) { [isMaximized, isMinimized, setContainerSize, max, min, initialSize] ); + const onDoubleClick = useCallback(() => { + setContainerSize(initialSize); + }, [initialSize]); + if (isLeftRight) { const {left: a, right: b} = props; diff --git a/static/app/utils/useResizableDrawer.tsx b/static/app/utils/useResizableDrawer.tsx index 9be2f561d96f3c..b894f7d6d3b265 100644 --- a/static/app/utils/useResizableDrawer.tsx +++ b/static/app/utils/useResizableDrawer.tsx @@ -35,18 +35,10 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { * handle. */ isHeld: boolean; - /** - * Apply this to include 'reset' functionality on the drag handle - */ - onDoubleClick: React.MouseEventHandler; /** * Apply to the drag handle element */ onMouseDown: React.MouseEventHandler; - /** - * Call this function to manually set the size of the drawer. - */ - setSize: (newSize: number, userEvent?: boolean) => void; } { const rafIdRef = useRef(null); const currentMouseVectorRaf = useRef<[number, number] | null>(null); @@ -139,9 +131,5 @@ export function useResizableDrawer(options: UseResizableDrawerOptions): { [onMouseMove, onMouseUp] ); - const onDoubleClick = useCallback(() => { - updateSize(options.initialSize, true); - }, [updateSize, options.initialSize]); - - return {isHeld, onMouseDown, onDoubleClick, setSize: updateSize}; + return {isHeld, onMouseDown}; } diff --git a/static/app/views/replays/detail/network/details/index.tsx b/static/app/views/replays/detail/network/details/index.tsx index cfbe24c02bee05..20c0097bf463f0 100644 --- a/static/app/views/replays/detail/network/details/index.tsx +++ b/static/app/views/replays/detail/network/details/index.tsx @@ -13,6 +13,7 @@ type Props = { isSetup: boolean; item: null | SpanFrame; onClose: () => void; + onDoubleClick: () => void; projectId: undefined | string; startTimestampMs: number; } & Omit, 'size'>;