diff --git a/public/app/features/explore/Logs/Logs.test.tsx b/public/app/features/explore/Logs/Logs.test.tsx index 1770198d7f8..8fcc18bf80b 100644 --- a/public/app/features/explore/Logs/Logs.test.tsx +++ b/public/app/features/explore/Logs/Logs.test.tsx @@ -169,6 +169,79 @@ describe('Logs', () => { return render(getComponent(partialProps, logs)); }; + describe('scrolling behavior', () => { + let originalInnerHeight: number; + beforeEach(() => { + originalInnerHeight = window.innerHeight; + window.innerHeight = 1000; + window.HTMLElement.prototype.scrollIntoView = jest.fn(); + window.HTMLElement.prototype.scroll = jest.fn(); + }); + afterEach(() => { + window.innerHeight = originalInnerHeight; + }); + + describe('when `exploreScrollableLogsContainer` is set', () => { + let featureToggle: boolean | undefined; + beforeEach(() => { + featureToggle = config.featureToggles.exploreScrollableLogsContainer; + config.featureToggles.exploreScrollableLogsContainer = true; + }); + afterEach(() => { + config.featureToggles.exploreScrollableLogsContainer = featureToggle; + jest.clearAllMocks(); + }); + + it('should call `this.state.logsContainer.scroll`', () => { + const scrollIntoViewSpy = jest.spyOn(window.HTMLElement.prototype, 'scrollIntoView'); + jest.spyOn(window.HTMLElement.prototype, 'scrollTop', 'get').mockReturnValue(920); + const scrollSpy = jest.spyOn(window.HTMLElement.prototype, 'scroll'); + + const logs = []; + for (let i = 0; i < 50; i++) { + logs.push(makeLog({ uid: `uid${i}`, rowId: `id${i}`, timeEpochMs: i })); + } + + setup({ panelState: { logs: { id: 'uid47' } } }, logs); + + expect(scrollIntoViewSpy).toBeCalledTimes(1); + // element.getBoundingClientRect().top will always be 0 for jsdom + // calc will be `this.state.logsContainer.scrollTop - window.innerHeight / 2` -> 920 - 500 = 420 + expect(scrollSpy).toBeCalledWith({ behavior: 'smooth', top: 420 }); + }); + }); + + describe('when `exploreScrollableLogsContainer` is not set', () => { + let featureToggle: boolean | undefined; + beforeEach(() => { + featureToggle = config.featureToggles.exploreScrollableLogsContainer; + config.featureToggles.exploreScrollableLogsContainer = false; + }); + afterEach(() => { + config.featureToggles.exploreScrollableLogsContainer = featureToggle; + }); + + it('should call `scrollElement.scroll`', () => { + const logs = []; + for (let i = 0; i < 50; i++) { + logs.push(makeLog({ uid: `uid${i}`, rowId: `id${i}`, timeEpochMs: i })); + } + const scrollElementMock = { + scroll: jest.fn(), + scrollTop: 920, + }; + setup( + { scrollElement: scrollElementMock as unknown as HTMLDivElement, panelState: { logs: { id: 'uid47' } } }, + logs + ); + + // element.getBoundingClientRect().top will always be 0 for jsdom + // calc will be `scrollElement.scrollTop - window.innerHeight / 2` -> 920 - 500 = 420 + expect(scrollElementMock.scroll).toBeCalledWith({ behavior: 'smooth', top: 420 }); + }); + }); + }); + it('should render logs', () => { setup(); const logsSection = screen.getByTestId('logRows'); diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index 4a367b3b7fd..ca62766192e 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -117,6 +117,7 @@ interface State { contextRow?: LogRowModel; tableFrame?: DataFrame; visualisationType?: VisualisationType; + logsContainer?: HTMLDivElement; } const scrollableLogsContainer = config.featureToggles.exploreScrollableLogsContainer; @@ -144,7 +145,6 @@ class UnthemedLogs extends PureComponent { cancelFlippingTimer?: number; topLogsRef = createRef(); logsVolumeEventBus: EventBus; - logsContainer = createRef(); state: State = { showLabels: store.getBool(SETTINGS_KEYS.showLabels, false), @@ -161,6 +161,7 @@ class UnthemedLogs extends PureComponent { contextRow: undefined, tableFrame: undefined, visualisationType: 'logs', + logsContainer: undefined, }; constructor(props: Props) { @@ -204,6 +205,10 @@ class UnthemedLogs extends PureComponent { } }; + onLogsContainerRef = (node: HTMLDivElement) => { + this.setState({ logsContainer: node }); + }; + onChangeLogsSortOrder = () => { this.setState({ isFlipping: true }); // we are using setTimeout here to make sure that disabled button is rendered before the rendering of reordered logs @@ -406,11 +411,11 @@ class UnthemedLogs extends PureComponent { scrollIntoView = (element: HTMLElement) => { if (config.featureToggles.exploreScrollableLogsContainer) { - this.scrollToTopLogs(); - if (this.logsContainer.current) { - this.logsContainer.current.scroll({ + if (this.state.logsContainer) { + this.topLogsRef.current?.scrollIntoView(); + this.state.logsContainer.scroll({ behavior: 'smooth', - top: this.logsContainer.current.scrollTop + element.getBoundingClientRect().top - window.innerHeight / 2, + top: this.state.logsContainer.scrollTop + element.getBoundingClientRect().top - window.innerHeight / 2, }); } @@ -456,8 +461,8 @@ class UnthemedLogs extends PureComponent { scrollToTopLogs = () => { if (config.featureToggles.exploreScrollableLogsContainer) { - if (this.logsContainer.current) { - this.logsContainer.current.scroll({ + if (this.state.logsContainer) { + this.state.logsContainer.scroll({ behavior: 'auto', top: 0, }); @@ -688,7 +693,7 @@ class UnthemedLogs extends PureComponent { )} {this.state.visualisationType === 'logs' && hasData && ( -
+
{ permalinkedRowId={this.props.panelState?.logs?.id} scrollIntoView={this.scrollIntoView} isFilterLabelActive={this.props.isFilterLabelActive} + containerRendered={!!this.state.logsContainer} />
)} diff --git a/public/app/features/logs/components/LogRow.test.tsx b/public/app/features/logs/components/LogRow.test.tsx index d0a613db42a..c8fc50d4d9b 100644 --- a/public/app/features/logs/components/LogRow.test.tsx +++ b/public/app/features/logs/components/LogRow.test.tsx @@ -61,7 +61,7 @@ describe('LogRow', () => { describe('with permalinking', () => { it('reports via feature tracking when log line matches', () => { const scrollIntoView = jest.fn(); - setup({ permalinkedRowId: 'log-row-id', scrollIntoView }); + setup({ permalinkedRowId: 'log-row-id', scrollIntoView, containerRendered: true }); expect(reportInteraction).toHaveBeenCalledWith('grafana_explore_logs_permalink_opened', { logRowUid: 'log-row-id', datasourceType: 'unknown', @@ -70,7 +70,11 @@ describe('LogRow', () => { }); it('highlights row with same permalink-id', () => { - const { container } = setup({ permalinkedRowId: 'log-row-id' }); + const { container } = setup({ + permalinkedRowId: 'log-row-id', + containerRendered: true, + scrollIntoView: jest.fn(), + }); const row = container.querySelector('tr'); expect(row).toHaveStyle( `background-color: ${tinycolor(theme.colors.info.transparent).setAlpha(0.25).toString()}` @@ -78,7 +82,12 @@ describe('LogRow', () => { }); it('does not highlight row details with different permalink-id', async () => { - const { container } = setup({ permalinkedRowId: 'log-row-id', enableLogDetails: true }); + const { container } = setup({ + permalinkedRowId: 'log-row-id', + enableLogDetails: true, + containerRendered: true, + scrollIntoView: jest.fn(), + }); const row = container.querySelector('tr'); await userEvent.click(row!); const allRows = container.querySelectorAll('tr'); @@ -101,22 +110,28 @@ describe('LogRow', () => { it('calls `scrollIntoView` if permalink matches', () => { const scrollIntoView = jest.fn(); - setup({ permalinkedRowId: 'log-row-id', scrollIntoView }); + setup({ permalinkedRowId: 'log-row-id', scrollIntoView, containerRendered: true }); expect(scrollIntoView).toHaveBeenCalled(); }); it('does not call `scrollIntoView` if permalink does not match', () => { const scrollIntoView = jest.fn(); - setup({ permalinkedRowId: 'wrong-log-row-id', scrollIntoView }); + setup({ permalinkedRowId: 'wrong-log-row-id', scrollIntoView, containerRendered: true }); expect(scrollIntoView).not.toHaveBeenCalled(); }); it('calls `scrollIntoView` once', async () => { const scrollIntoView = jest.fn(); - setup({ permalinkedRowId: 'log-row-id', scrollIntoView }); + setup({ permalinkedRowId: 'log-row-id', scrollIntoView, containerRendered: true }); await userEvent.hover(screen.getByText('test123')); expect(scrollIntoView).toHaveBeenCalledTimes(1); }); + + it('does not call `scrollIntoView` if permalink matches but container is not rendered yet', () => { + const scrollIntoView = jest.fn(); + setup({ permalinkedRowId: 'log-row-id', scrollIntoView, containerRendered: false }); + expect(scrollIntoView).not.toHaveBeenCalled(); + }); }); it('should render the menu cell on mouse over', async () => { diff --git a/public/app/features/logs/components/LogRow.tsx b/public/app/features/logs/components/LogRow.tsx index 3fd07d18e35..8c565c5f4a5 100644 --- a/public/app/features/logs/components/LogRow.tsx +++ b/public/app/features/logs/components/LogRow.tsx @@ -46,6 +46,7 @@ interface Props extends Themeable2 { onPinLine?: (row: LogRowModel) => void; onUnpinLine?: (row: LogRowModel) => void; pinned?: boolean; + containerRendered?: boolean; } interface State { @@ -135,7 +136,7 @@ class UnThemedLogRow extends PureComponent { } scrollToLogRow = (prevState: State, mounted = false) => { - const { row, permalinkedRowId, scrollIntoView } = this.props; + const { row, permalinkedRowId, scrollIntoView, containerRendered } = this.props; if (permalinkedRowId !== row.uid) { // only set the new state if the row is not permalinked anymore or if the component was mounted. @@ -145,11 +146,9 @@ class UnThemedLogRow extends PureComponent { return; } - if (!this.state.permalinked) { + if (!this.state.permalinked && containerRendered && this.logLineRef.current && scrollIntoView) { // at this point this row is the permalinked row, so we need to scroll to it and highlight it if possible. - if (this.logLineRef.current && scrollIntoView) { - scrollIntoView(this.logLineRef.current); - } + scrollIntoView(this.logLineRef.current); reportInteraction('grafana_explore_logs_permalink_opened', { datasourceType: row.datasourceType ?? 'unknown', logRowUid: row.uid, diff --git a/public/app/features/logs/components/LogRows.tsx b/public/app/features/logs/components/LogRows.tsx index 5ba3f224f3b..bce6a961241 100644 --- a/public/app/features/logs/components/LogRows.tsx +++ b/public/app/features/logs/components/LogRows.tsx @@ -52,6 +52,7 @@ export interface Props extends Themeable2 { scrollIntoView?: (element: HTMLElement) => void; isFilterLabelActive?: (key: string, value: string) => Promise; pinnedRowId?: string; + containerRendered?: boolean; } interface State {