From 0dfaca0f2eb5469351d13c1b63c914b3d23998f6 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Thu, 17 Oct 2024 15:00:56 -0700 Subject: [PATCH 1/4] fix(issues): Avoid streamline issue layout rerenders the page seems to rerender when scrolling the page due to setNav + isStuck triggering dispatch. by containing it in its own component we can avoid rerendering --- .../issueDetails/streamline/eventDetails.tsx | 58 ++++++++++++------- 1 file changed, 38 insertions(+), 20 deletions(-) diff --git a/static/app/views/issueDetails/streamline/eventDetails.tsx b/static/app/views/issueDetails/streamline/eventDetails.tsx index 155e3d6584d11a..87903e29b50c45 100644 --- a/static/app/views/issueDetails/streamline/eventDetails.tsx +++ b/static/app/views/issueDetails/streamline/eventDetails.tsx @@ -10,6 +10,8 @@ import {DatePageFilter} from 'sentry/components/organizations/datePageFilter'; import {EnvironmentPageFilter} from 'sentry/components/organizations/environmentPageFilter'; import {t} from 'sentry/locale'; import {space} from 'sentry/styles/space'; +import type {Event} from 'sentry/types/event'; +import type {Group} from 'sentry/types/group'; import type {MultiSeriesEventsStats} from 'sentry/types/organization'; import {useIsStuck} from 'sentry/utils/useIsStuck'; import {useLocation} from 'sentry/utils/useLocation'; @@ -44,14 +46,10 @@ export function EventDetails({ event, project, }: Required) { - const theme = useTheme(); const location = useLocation(); const navigate = useNavigate(); const {selection} = usePageFilters(); - const isScreenMedium = useMedia(`(max-width: ${theme.breakpoints.medium})`); const {environments} = selection; - const [nav, setNav] = useState(null); - const isStuck = useIsStuck(nav); const {eventDetails, dispatch} = useEventDetailsReducer(); const searchQuery = useEventQuery({group}); @@ -70,15 +68,6 @@ export function EventDetails({ }, }); - useLayoutEffect(() => { - const navHeight = nav?.offsetHeight ?? 0; - const sidebarHeight = isScreenMedium ? theme.sidebar.mobileHeightNumber : 0; - dispatch({ - type: 'UPDATE_DETAILS', - state: {navScrollMargin: navHeight + sidebarHeight}, - }); - }, [nav, isScreenMedium, dispatch, theme.sidebar.mobileHeightNumber]); - return ( @@ -135,13 +124,7 @@ export function EventDetails({ message={t('There was an error loading the event content')} > - + @@ -159,6 +142,41 @@ export function EventDetails({ ); } +function StickyEventNav({ + event, + group, + searchQuery, +}: { + event: Event; + group: Group; + searchQuery: string; +}) { + const theme = useTheme(); + const [nav, setNav] = useState(null); + const isStuck = useIsStuck(nav); + const isScreenMedium = useMedia(`(max-width: ${theme.breakpoints.medium})`); + const {dispatch} = useEventDetailsReducer(); + + useLayoutEffect(() => { + const navHeight = nav?.offsetHeight ?? 0; + const sidebarHeight = isScreenMedium ? theme.sidebar.mobileHeightNumber : 0; + dispatch({ + type: 'UPDATE_DETAILS', + state: {navScrollMargin: navHeight + sidebarHeight}, + }); + }, [nav, isScreenMedium, dispatch, theme.sidebar.mobileHeightNumber]); + + return ( + + ); +} + const SearchFilter = styled(EventSearch)` border-radius: ${p => p.theme.borderRadius}; `; From 3d37fd58d8d9b815b22ee873dd010c83788a17f9 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Thu, 17 Oct 2024 15:27:05 -0700 Subject: [PATCH 2/4] avoid nav rerender --- static/app/views/issueDetails/streamline/eventDetails.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/static/app/views/issueDetails/streamline/eventDetails.tsx b/static/app/views/issueDetails/streamline/eventDetails.tsx index 87903e29b50c45..ca5b98a82bd9de 100644 --- a/static/app/views/issueDetails/streamline/eventDetails.tsx +++ b/static/app/views/issueDetails/streamline/eventDetails.tsx @@ -158,7 +158,11 @@ function StickyEventNav({ const {dispatch} = useEventDetailsReducer(); useLayoutEffect(() => { - const navHeight = nav?.offsetHeight ?? 0; + if (!nav) { + return; + } + + const navHeight = nav.offsetHeight ?? 0; const sidebarHeight = isScreenMedium ? theme.sidebar.mobileHeightNumber : 0; dispatch({ type: 'UPDATE_DETAILS', From 1556783e4a2c1a05cfd058dae5ad31fc3ec09fe5 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Thu, 17 Oct 2024 15:44:26 -0700 Subject: [PATCH 3/4] pass dispatch down --- .../app/views/issueDetails/streamline/eventDetails.tsx | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/static/app/views/issueDetails/streamline/eventDetails.tsx b/static/app/views/issueDetails/streamline/eventDetails.tsx index ca5b98a82bd9de..3c8f83b9f15a71 100644 --- a/static/app/views/issueDetails/streamline/eventDetails.tsx +++ b/static/app/views/issueDetails/streamline/eventDetails.tsx @@ -124,7 +124,12 @@ export function EventDetails({ message={t('There was an error loading the event content')} > - + @@ -146,7 +151,9 @@ function StickyEventNav({ event, group, searchQuery, + dispatch, }: { + dispatch: ReturnType['dispatch']; event: Event; group: Group; searchQuery: string; @@ -155,7 +162,6 @@ function StickyEventNav({ const [nav, setNav] = useState(null); const isStuck = useIsStuck(nav); const isScreenMedium = useMedia(`(max-width: ${theme.breakpoints.medium})`); - const {dispatch} = useEventDetailsReducer(); useLayoutEffect(() => { if (!nav) { From c1510d939e00b39aadc8a9e88e650dd155646006 Mon Sep 17 00:00:00 2001 From: Scott Cooper Date: Fri, 18 Oct 2024 13:25:33 -0700 Subject: [PATCH 4/4] instead of passing it down, use context --- .../views/issueDetails/streamline/eventDetails.tsx | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/static/app/views/issueDetails/streamline/eventDetails.tsx b/static/app/views/issueDetails/streamline/eventDetails.tsx index 3c8f83b9f15a71..f2b0a468c3ed87 100644 --- a/static/app/views/issueDetails/streamline/eventDetails.tsx +++ b/static/app/views/issueDetails/streamline/eventDetails.tsx @@ -24,6 +24,7 @@ import { } from 'sentry/views/issueDetails/groupEventDetails/groupEventDetailsContent'; import { EventDetailsContext, + useEventDetails, useEventDetailsReducer, } from 'sentry/views/issueDetails/streamline/context'; import {EventGraph} from 'sentry/views/issueDetails/streamline/eventGraph'; @@ -124,12 +125,7 @@ export function EventDetails({ message={t('There was an error loading the event content')} > - + @@ -151,9 +147,7 @@ function StickyEventNav({ event, group, searchQuery, - dispatch, }: { - dispatch: ReturnType['dispatch']; event: Event; group: Group; searchQuery: string; @@ -162,6 +156,7 @@ function StickyEventNav({ const [nav, setNav] = useState(null); const isStuck = useIsStuck(nav); const isScreenMedium = useMedia(`(max-width: ${theme.breakpoints.medium})`); + const {dispatch} = useEventDetails(); useLayoutEffect(() => { if (!nav) {