Skip to content

Commit 2773bdf

Browse files
JoshuaKGoldbergJoshuaKGoldberg
committed
feat(logs): switch LogsInfiniteTable to toggle-able bound scrolling
1 parent 5795cd1 commit 2773bdf

File tree

14 files changed

+156
-76
lines changed

14 files changed

+156
-76
lines changed

‎static/app/components/events/ourlogs/ourlogsDrawer.tsx‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {useMemo, useRef} from 'react';
1+
import {useMemo} from 'react';
22
import moment from 'moment-timezone';
33

44
import {ProjectAvatar} from '@sentry/scraps/avatar';
@@ -83,7 +83,6 @@ export function OurlogsDrawer({
8383
const searchQueryBuilderProps = useTraceItemSearchQueryBuilderProps(
8484
tracesItemSearchQueryBuilderProps
8585
);
86-
const containerRef = useRef<HTMLDivElement>(null);
8786

8887
const additionalData = useMemo(
8988
() => ({
@@ -156,11 +155,10 @@ export function OurlogsDrawer({
156155
)}
157156
</Flex>
158157
</EventNavigator>
159-
<EventDrawerBody ref={containerRef}>
158+
<EventDrawerBody>
160159
<Stack gap="xl">
161160
<LogsInfiniteTable
162161
embedded
163-
scrollContainer={containerRef}
164162
embeddedOptions={embeddedOptions}
165163
additionalData={additionalData}
166164
/>

‎static/app/components/events/ourlogs/ourlogsSection.spec.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ jest.mock('@tanstack/react-virtual', () => {
4343
{key: '3', index: 2, start: 100, end: 150, lane: 0},
4444
]),
4545
getTotalSize: jest.fn().mockReturnValue(150),
46+
measure: jest.fn(),
4647
scrollToIndex: jest.fn(),
4748
options: {
4849
scrollMargin: 0,

‎static/app/views/explore/logs/content.spec.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {createLogFixtures, initializeLogsTest} from 'sentry-fixture/log';
22
import {ProjectKeysFixture} from 'sentry-fixture/projectKeys';
33
import {TeamFixture} from 'sentry-fixture/team';
44
import {TimeSeriesFixture} from 'sentry-fixture/timeSeries';
5+
import {mockGetBoundingClientRect} from 'sentry-fixture/virtualization';
56

67
import {
78
render,
@@ -21,6 +22,8 @@ import type {OurLogsResponseItem} from 'sentry/views/explore/logs/types';
2122

2223
import LogsPage from './content';
2324

25+
beforeEach(mockGetBoundingClientRect);
26+
2427
describe('LogsPage', () => {
2528
let organization: Organization;
2629
let project: Project;

‎static/app/views/explore/logs/logsTab.spec.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {initializeLogsTest} from 'sentry-fixture/log';
22
import {TimeSeriesFixture} from 'sentry-fixture/timeSeries';
3+
import {mockGetBoundingClientRect} from 'sentry-fixture/virtualization';
34

45
import {render, screen, userEvent, waitFor} from 'sentry-test/reactTestingLibrary';
56

@@ -28,6 +29,8 @@ const datePageFilterProps: DatePageFilterProps = {
2829
}),
2930
};
3031

32+
beforeEach(mockGetBoundingClientRect);
33+
3134
describe('LogsTabContent', () => {
3235
const {organization, project, setupPageFilters} = initializeLogsTest();
3336

‎static/app/views/explore/logs/styles.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,7 @@ export const LogTableBodyCell = styled(TableBodyCell)`
127127

128128
export const LogTableBody = styled(TableBody)<{
129129
disableBodyPadding?: boolean;
130+
expanded?: boolean;
130131
showHeader?: boolean;
131132
}>`
132133
${p =>
@@ -138,6 +139,10 @@ export const LogTableBody = styled(TableBody)<{
138139
padding-top: ${p.theme.space.md};
139140
padding-bottom: ${p.theme.space.md};
140141
`}
142+
overflow-y: scroll;
143+
max-height: ${p =>
144+
p.expanded ? `calc(100vh - ${GRID_BODY_ROW_HEIGHT * 1.5}px)` : '50vh'};
145+
min-height: 5rem;
141146
`;
142147

143148
export const LogDetailTableBodyCell = styled(TableBodyCell)`
@@ -294,6 +299,7 @@ export const LogsItemContainer = styled('div')`
294299
flex: 1 1 auto;
295300
margin-top: ${p => p.theme.space.md};
296301
margin-bottom: ${p => p.theme.space.md};
302+
position: relative;
297303
`;
298304

299305
export const LogsTableActionsContainer = styled(LogsItemContainer)`

‎static/app/views/explore/logs/tables/logsInfiniteTable.spec.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ jest.mock('@tanstack/react-virtual', () => {
6666
{key: '3', index: 2, start: 100, end: 150, lane: 0},
6767
]),
6868
getTotalSize: jest.fn().mockReturnValue(150),
69+
measure: jest.fn(),
6970
options: {
7071
scrollMargin: 0,
7172
},

‎static/app/views/explore/logs/tables/logsInfiniteTable.tsx‎

Lines changed: 48 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1-
import type {CSSProperties, RefObject} from 'react';
2-
import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react';
1+
import {
2+
Fragment,
3+
useCallback,
4+
useEffect,
5+
useLayoutEffect,
6+
useMemo,
7+
useRef,
8+
useState,
9+
type CSSProperties,
10+
type RefObject,
11+
} from 'react';
312
import styled from '@emotion/styled';
413
import * as Sentry from '@sentry/react';
514
import type {Virtualizer} from '@tanstack/react-virtual';
6-
import {useVirtualizer, useWindowVirtualizer} from '@tanstack/react-virtual';
15+
import {useVirtualizer} from '@tanstack/react-virtual';
716

817
import {Button} from '@sentry/scraps/button';
918
import {Flex, Stack} from '@sentry/scraps/layout';
@@ -49,6 +58,7 @@ import {
4958
LogTableRow,
5059
} from 'sentry/views/explore/logs/styles';
5160
import {LogRowContent} from 'sentry/views/explore/logs/tables/logsTableRow';
61+
import {useExpandoButton} from 'sentry/views/explore/logs/tables/useExpandoButton';
5262
import {
5363
OurLogKnownFieldKey,
5464
type OurLogsResponseItem,
@@ -95,14 +105,12 @@ type LogsTableProps = {
95105
filteredItems: OurLogsResponseItem[];
96106
};
97107
numberAttributes?: TagCollection;
98-
scrollContainer?: React.RefObject<HTMLElement | null>;
99108
stringAttributes?: TagCollection;
100109
};
101110

102111
const {info, fmt} = Sentry.logger;
103112

104113
const LOGS_GRID_SCROLL_PIXEL_REVERSE_THRESHOLD = LOGS_GRID_BODY_ROW_HEIGHT * 2; // If you are less than this number of pixels from the top of the table while scrolling backward, fetch the previous page.
105-
const LOGS_OVERSCAN_AMOUNT = 50; // How many items to render beyond the visible area.
106114

107115
export function LogsInfiniteTable({
108116
embedded = false,
@@ -111,7 +119,6 @@ export function LogsInfiniteTable({
111119
numberAttributes,
112120
stringAttributes,
113121
booleanAttributes,
114-
scrollContainer,
115122
embeddedStyling,
116123
embeddedOptions,
117124
additionalData,
@@ -256,23 +263,20 @@ export function LogsInfiniteTable({
256263
// eslint-disable-next-line react-hooks/exhaustive-deps
257264
}, [searchString, localOnlyItemFilters?.filterText]);
258265

259-
const windowVirtualizer = useWindowVirtualizer({
260-
count: data?.length ?? 0,
261-
estimateSize,
262-
overscan: LOGS_OVERSCAN_AMOUNT,
263-
getItemKey: (index: number) => data?.[index]?.[OurLogKnownFieldKey.ID] ?? index,
264-
scrollMargin: tableBodyRef.current?.offsetTop ?? 0,
265-
});
266+
const {expanded, expando} = useExpandoButton();
266267

267-
const containerVirtualizer = useVirtualizer({
268+
const virtualizer = useVirtualizer<HTMLElement, Element>({
268269
count: data?.length ?? 0,
269270
estimateSize,
270-
overscan: LOGS_OVERSCAN_AMOUNT,
271-
getScrollElement: () => scrollContainer?.current ?? null,
271+
overscan: expanded ? 25 : 10,
272+
getScrollElement: () => tableBodyRef?.current,
272273
getItemKey: (index: number) => data?.[index]?.[OurLogKnownFieldKey.ID] ?? index,
273274
});
274275

275-
const virtualizer = scrollContainer?.current ? containerVirtualizer : windowVirtualizer;
276+
useLayoutEffect(() => {
277+
virtualizer.measure();
278+
}, [expanded, virtualizer]);
279+
276280
const virtualItems = virtualizer.getVirtualItems();
277281

278282
const firstItem = virtualItems[0]?.start;
@@ -293,22 +297,22 @@ export function LogsInfiniteTable({
293297
useEffect(() => {
294298
if (
295299
pseudoRowIndex !== -1 &&
296-
scrollContainer?.current &&
300+
tableBodyRef?.current &&
297301
!additionalData?.scrollToDisabled
298302
) {
299303
setTimeout(() => {
300304
const scrollToIndex =
301305
pseudoRowIndex === -2 ? baseDataLength.current : pseudoRowIndex;
302-
containerVirtualizer.scrollToIndex(scrollToIndex, {
306+
virtualizer.scrollToIndex(scrollToIndex, {
303307
behavior: 'smooth',
304308
align: 'center',
305309
});
306310
}, 100);
307311
}
308312
}, [
309313
pseudoRowIndex,
310-
containerVirtualizer,
311-
scrollContainer,
314+
virtualizer,
315+
tableBodyRef,
312316
baseDataLength,
313317
additionalData?.scrollToDisabled,
314318
]);
@@ -337,9 +341,7 @@ export function LogsInfiniteTable({
337341
]
338342
: [0, 0];
339343

340-
const {scrollDirection, scrollOffset, isScrolling} = scrollContainer
341-
? containerVirtualizer
342-
: virtualizer;
344+
const {scrollDirection, scrollOffset, isScrolling} = virtualizer;
343345

344346
useEffect(() => {
345347
if (isFunctionScrolling && !isScrolling && scrollOffset === 0) {
@@ -452,6 +454,7 @@ export function LogsInfiniteTable({
452454

453455
return (
454456
<Fragment>
457+
{expando}
455458
<Table
456459
ref={tableRef}
457460
style={initialTableStyles}
@@ -473,6 +476,7 @@ export function LogsInfiniteTable({
473476
showHeader={!embedded}
474477
ref={tableBodyRef}
475478
disableBodyPadding={embeddedStyling?.disableBodyPadding}
479+
expanded={expanded}
476480
>
477481
{paddingTop > 0 && (
478482
<TableRow>
@@ -539,24 +543,26 @@ export function LogsInfiniteTable({
539543
)}
540544
</LogTableBody>
541545
</Table>
542-
<FloatingBackToTopContainer
543-
inReplay={!!embeddedOptions?.replay}
544-
tableLeft={tableRef.current?.getBoundingClientRect().left ?? 0}
545-
tableWidth={tableRef.current?.getBoundingClientRect().width ?? 0}
546-
>
547-
{!embeddedOptions?.replay && (
548-
<BackToTopButton
549-
virtualizer={virtualizer}
550-
hidden={
551-
isPending || ((firstItemIndex ?? 0) === 0 && (scrollOffset ?? 0) < 550)
552-
}
553-
setIsFunctionScrolling={setIsFunctionScrolling}
554-
/>
555-
)}
556-
{embeddedOptions?.replay && showJumpUpButton ? (
557-
<JumpButtons jump="up" onClick={onClickToJump} tableHeaderHeight={0} />
558-
) : null}
559-
</FloatingBackToTopContainer>
546+
{expanded && (
547+
<FloatingBackToTopContainer
548+
inReplay={!!embeddedOptions?.replay}
549+
tableLeft={tableRef.current?.getBoundingClientRect().left ?? 0}
550+
tableWidth={tableRef.current?.getBoundingClientRect().width ?? 0}
551+
>
552+
{!embeddedOptions?.replay && (
553+
<BackToTopButton
554+
virtualizer={virtualizer}
555+
hidden={
556+
isPending || ((firstItemIndex ?? 0) === 0 && (scrollOffset ?? 0) < 550)
557+
}
558+
setIsFunctionScrolling={setIsFunctionScrolling}
559+
/>
560+
)}
561+
{embeddedOptions?.replay && showJumpUpButton ? (
562+
<JumpButtons jump="up" onClick={onClickToJump} tableHeaderHeight={0} />
563+
) : null}
564+
</FloatingBackToTopContainer>
565+
)}
560566
<FloatingBottomContainer
561567
tableWidth={tableRef.current?.getBoundingClientRect().width ?? 0}
562568
>

‎static/app/views/explore/logs/tables/useExpandoButton.tsx‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import {useCallback, useLayoutEffect, useRef, useState} from 'react';
2+
import styled from '@emotion/styled';
3+
4+
import {Button} from '@sentry/scraps/button';
5+
6+
import {IconContract, IconExpand} from 'sentry/icons';
7+
import {t} from 'sentry/locale';
8+
import {TableActionButton} from 'sentry/views/explore/components/tableActionButton';
9+
10+
export function useExpandoButton() {
11+
const [expanded, setExpanded] = useState(false);
12+
const ref = useRef<HTMLButtonElement>(null);
13+
14+
const [Icon, text] = expanded
15+
? [IconContract, t('Collapse')]
16+
: [IconExpand, t('Expand')];
17+
18+
const toggleExpanded = useCallback(() => {
19+
setExpanded(previousExpanded => !previousExpanded);
20+
}, []);
21+
22+
useLayoutEffect(() => {
23+
if (expanded) {
24+
ref.current?.scrollIntoView({
25+
block: 'start',
26+
});
27+
}
28+
}, [expanded]);
29+
30+
const buttonProps = {
31+
'aria-label': text,
32+
icon: <Icon />,
33+
onClick: toggleExpanded,
34+
ref,
35+
size: 'sm',
36+
} as const;
37+
38+
return {
39+
expanded,
40+
expando: (
41+
<ExpandoContainer>
42+
<TableActionButton
43+
mobile={<ExpandoButton {...buttonProps} />}
44+
desktop={<ExpandoButton {...buttonProps}>{text}</ExpandoButton>}
45+
/>
46+
</ExpandoContainer>
47+
),
48+
};
49+
}
50+
51+
const ExpandoButton = styled(Button)`
52+
scroll-margin-top: 0.75rem;
53+
`;
54+
55+
const ExpandoContainer = styled('div')`
56+
position: absolute;
57+
top: 0.5rem;
58+
right: 0.5rem;
59+
z-index: 1;
60+
`;

‎static/app/views/performance/newTraceDetails/index.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ function TraceViewImpl({traceSlug}: {traceSlug: string}) {
134134
logs: logsData,
135135
traceId: traceSlug,
136136
});
137-
const traceInnerLayoutRef = useRef<HTMLDivElement>(null);
138137

139138
const {tabOptions, currentTab, onTabChange} = useTraceLayoutTabs({
140139
tree,
@@ -159,7 +158,7 @@ function TraceViewImpl({traceSlug}: {traceSlug: string}) {
159158
logs={logsData}
160159
metrics={metricsData}
161160
/>
162-
<TraceInnerLayout ref={traceInnerLayoutRef}>
161+
<TraceInnerLayout>
163162
<ErrorsOnlyWarnings
164163
tree={tree}
165164
traceSlug={traceSlug}
@@ -196,9 +195,7 @@ function TraceViewImpl({traceSlug}: {traceSlug: string}) {
196195
{currentTab === TraceLayoutTabKeys.PROFILES ? (
197196
<TraceProfiles tree={tree} />
198197
) : null}
199-
{currentTab === TraceLayoutTabKeys.LOGS ? (
200-
<TraceViewLogsSection scrollContainer={traceInnerLayoutRef} />
201-
) : null}
198+
{currentTab === TraceLayoutTabKeys.LOGS ? <TraceViewLogsSection /> : null}
202199
{currentTab === TraceLayoutTabKeys.METRICS ? (
203200
<TraceViewMetricsProviderWrapper traceSlug={traceSlug}>
204201
<TraceViewMetricsSection />

0 commit comments

Comments
 (0)