From e07417dca34962e78f7f56c0da37e4eb6c1fff59 Mon Sep 17 00:00:00 2001 From: Matias Chomicki Date: Tue, 23 Jan 2024 17:52:38 +0100 Subject: [PATCH] Infinite scroll: share loader with log context + small fixes (#81067) * Loading indicator: refactor * Infinite scroll: use ref to keep the last scroll * Infinite scroll: share loading indicator with log context * Infinite scrolling: override state * Infinite scroll: clean up imports and dependency array * Logs: disable order while loading * Infinite scroll: update ref before calling shouldLoadMore * Loading indicator: remove unused export * Logs order: move disabled prop to inline field * Formatting * Update betterer file --- .betterer.results | 6 ++--- public/app/features/explore/Logs/Logs.tsx | 8 +++++-- public/app/features/explore/state/query.ts | 3 ++- .../logs/components/InfiniteScroll.tsx | 22 ++++++++----------- .../{log-context => }/LoadingIndicator.tsx | 9 +++----- .../log-context/LogRowContextModal.tsx | 8 +++---- .../logs/components/log-context/types.ts | 1 - 7 files changed, 27 insertions(+), 30 deletions(-) rename public/app/features/logs/components/{log-context => }/LoadingIndicator.tsx (69%) delete mode 100644 public/app/features/logs/components/log-context/types.ts diff --git a/.betterer.results b/.betterer.results index edef07473db39..6ccf70dd6ec98 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3691,6 +3691,9 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "3"], [0, 0, 0, "Do not use any type assertions.", "4"] ], + "public/app/features/logs/components/LoadingIndicator.tsx:5381": [ + [0, 0, 0, "Styles should be written using objects.", "0"] + ], "public/app/features/logs/components/LogDetailsRow.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"], @@ -3763,9 +3766,6 @@ exports[`better eslint`] = { [0, 0, 0, "Styles should be written using objects.", "34"], [0, 0, 0, "Styles should be written using objects.", "35"] ], - "public/app/features/logs/components/log-context/LoadingIndicator.tsx:5381": [ - [0, 0, 0, "Styles should be written using objects.", "0"] - ], "public/app/features/logs/components/log-context/LogRowContextModal.tsx:5381": [ [0, 0, 0, "Styles should be written using objects.", "0"], [0, 0, 0, "Styles should be written using objects.", "1"], diff --git a/public/app/features/explore/Logs/Logs.tsx b/public/app/features/explore/Logs/Logs.tsx index 59b3a5313721f..c927254a478bc 100644 --- a/public/app/features/explore/Logs/Logs.tsx +++ b/public/app/features/explore/Logs/Logs.tsx @@ -754,9 +754,13 @@ class UnthemedLogs extends PureComponent {
- + { // For query splitting, otherwise duplicates results if (data.state !== LoadingState.Done) { - return of(queryResponse); + // While loading, return the previous response and override state, otherwise it's set to Done + return of({ ...queryResponse, state: LoadingState.Loading }); } return decorateData( combinePanelData(queryResponse, data), diff --git a/public/app/features/logs/components/InfiniteScroll.tsx b/public/app/features/logs/components/InfiniteScroll.tsx index ef42847808724..2a048c0f4ad9d 100644 --- a/public/app/features/logs/components/InfiniteScroll.tsx +++ b/public/app/features/logs/components/InfiniteScroll.tsx @@ -1,11 +1,12 @@ import { css } from '@emotion/css'; -import React, { ReactNode, useEffect, useState } from 'react'; +import React, { ReactNode, useEffect, useRef, useState } from 'react'; import { AbsoluteTimeRange, LogRowModel, TimeRange } from '@grafana/data'; import { convertRawToRange, isRelativeTime, isRelativeTimeRange } from '@grafana/data/src/datetime/rangeutil'; import { reportInteraction } from '@grafana/runtime'; import { LogsSortOrder, TimeZone } from '@grafana/schema'; -import { Spinner } from '@grafana/ui'; + +import { LoadingIndicator } from './LoadingIndicator'; export type Props = { children: ReactNode; @@ -32,7 +33,7 @@ export const InfiniteScroll = ({ const [lowerOutOfRange, setLowerOutOfRange] = useState(false); const [upperLoading, setUpperLoading] = useState(false); const [lowerLoading, setLowerLoading] = useState(false); - const [lastScroll, setLastScroll] = useState(scrollElement?.scrollTop || 0); + const lastScroll = useRef(scrollElement?.scrollTop || 0); useEffect(() => { setUpperOutOfRange(false); @@ -56,8 +57,8 @@ export const InfiniteScroll = ({ return; } event.stopImmediatePropagation(); - setLastScroll(scrollElement.scrollTop); - const scrollDirection = shouldLoadMore(event, scrollElement, lastScroll); + const scrollDirection = shouldLoadMore(event, scrollElement, lastScroll.current); + lastScroll.current = scrollElement.scrollTop; if (scrollDirection === ScrollDirection.NoScroll) { return; } else if (scrollDirection === ScrollDirection.Top) { @@ -110,7 +111,7 @@ export const InfiniteScroll = ({ scrollElement.removeEventListener('scroll', handleScroll); scrollElement.removeEventListener('wheel', handleScroll); }; - }, [lastScroll, loadMoreLogs, loading, range, rows, scrollElement, sortOrder, timeZone]); + }, [loadMoreLogs, loading, range, rows, scrollElement, sortOrder, timeZone]); // We allow "now" to move when using relative time, so we hide the message so it doesn't flash. const hideTopMessage = sortOrder === LogsSortOrder.Descending && isRelativeTime(range.raw.to); @@ -118,11 +119,11 @@ export const InfiniteScroll = ({ return ( <> - {upperLoading && loadingMessage} + {upperLoading && } {!hideTopMessage && upperOutOfRange && outOfRangeMessage} {children} {!hideBottomMessage && lowerOutOfRange && outOfRangeMessage} - {lowerLoading && loadingMessage} + {lowerLoading && } ); }; @@ -139,11 +140,6 @@ const outOfRangeMessage = ( End of the selected time range.
); -const loadingMessage = ( -
- -
-); enum ScrollDirection { Top = -1, diff --git a/public/app/features/logs/components/log-context/LoadingIndicator.tsx b/public/app/features/logs/components/LoadingIndicator.tsx similarity index 69% rename from public/app/features/logs/components/log-context/LoadingIndicator.tsx rename to public/app/features/logs/components/LoadingIndicator.tsx index 06a72775e9c14..c483b18c92fcd 100644 --- a/public/app/features/logs/components/log-context/LoadingIndicator.tsx +++ b/public/app/features/logs/components/LoadingIndicator.tsx @@ -3,17 +3,14 @@ import React from 'react'; import { Spinner } from '@grafana/ui'; -import { Place } from './types'; - // ideally we'd use `@grafana/ui/LoadingPlaceholder`, but that // one has a large margin-bottom. - type Props = { - place: Place; + adjective?: string; }; -export const LoadingIndicator = ({ place }: Props) => { - const text = place === 'above' ? 'Loading newer logs...' : 'Loading older logs...'; +export const LoadingIndicator = ({ adjective = 'newer' }: Props) => { + const text = `Loading ${adjective} logs...`; return (
diff --git a/public/app/features/logs/components/log-context/LogRowContextModal.tsx b/public/app/features/logs/components/log-context/LogRowContextModal.tsx index 8daf1cb5edd88..684b83700ee15 100644 --- a/public/app/features/logs/components/log-context/LogRowContextModal.tsx +++ b/public/app/features/logs/components/log-context/LogRowContextModal.tsx @@ -26,11 +26,10 @@ import { useDispatch } from 'app/types'; import { dataFrameToLogsModel } from '../../logsModel'; import { sortLogRows } from '../../utils'; +import { LoadingIndicator } from '../LoadingIndicator'; import { LogRows } from '../LogRows'; -import { LoadingIndicator } from './LoadingIndicator'; import { LogContextButtons } from './LogContextButtons'; -import { Place } from './types'; const getStyles = (theme: GrafanaTheme2) => { return { @@ -143,6 +142,7 @@ type Section = { loadingState: LoadingState; rows: LogRowModel[]; }; +type Place = 'above' | 'below'; type Context = Record; const makeEmptyContext = (): Context => ({ @@ -518,7 +518,7 @@ export const LogRowContextModal: React.FunctionComponent {loadingStateAbove !== LoadingState.Done && loadingStateAbove !== LoadingState.Error && (
- +
)} {loadingStateAbove === LoadingState.Error &&
Error loading log more logs.
} @@ -587,7 +587,7 @@ export const LogRowContextModal: React.FunctionComponent {loadingStateBelow !== LoadingState.Done && loadingStateBelow !== LoadingState.Error && (
- +
)} {loadingStateBelow === LoadingState.Error &&
Error loading log more logs.
} diff --git a/public/app/features/logs/components/log-context/types.ts b/public/app/features/logs/components/log-context/types.ts deleted file mode 100644 index 9f625102e423f..0000000000000 --- a/public/app/features/logs/components/log-context/types.ts +++ /dev/null @@ -1 +0,0 @@ -export type Place = 'above' | 'below';