From 9ace76a718ec194397ad3d3a83bdfc9d3be746fa Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 11 May 2021 22:10:45 +0200 Subject: [PATCH] Explore: Polish UI of logs navigation (#33910) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update UI * Update UI, return spinner * Add title to Scroll to top button * Update public/app/features/explore/Logs.tsx Co-authored-by: Gábor Farkas * Update public/app/features/explore/LogsNavigation.test.tsx Co-authored-by: Gábor Farkas * Remove unnecessary memoization * Update public/app/features/explore/LogsNavigationPages.tsx Co-authored-by: Gábor Farkas --- .../src/components/Logs/getLogRowStyles.ts | 1 + public/app/features/explore/Logs.tsx | 18 +- public/app/features/explore/LogsContainer.tsx | 16 +- .../features/explore/LogsNavigation.test.tsx | 48 ++-- .../app/features/explore/LogsNavigation.tsx | 220 +++++++----------- .../explore/LogsNavigationPages.test.tsx | 45 ++++ .../features/explore/LogsNavigationPages.tsx | 121 ++++++++++ 7 files changed, 311 insertions(+), 158 deletions(-) create mode 100644 public/app/features/explore/LogsNavigationPages.test.tsx create mode 100644 public/app/features/explore/LogsNavigationPages.tsx diff --git a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts index 816a61267fd..9fd2b6d2539 100644 --- a/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts +++ b/packages/grafana-ui/src/components/Logs/getLogRowStyles.ts @@ -48,6 +48,7 @@ export const getLogRowStyles = stylesFactory((theme: GrafanaTheme, logLevel?: Lo font-family: ${theme.typography.fontFamily.monospace}; font-size: ${theme.typography.size.sm}; width: 100%; + overflow: hidden; `, context: css` label: context; diff --git a/public/app/features/explore/Logs.tsx b/public/app/features/explore/Logs.tsx index 5bed83e26ec..d921ad91ef0 100644 --- a/public/app/features/explore/Logs.tsx +++ b/public/app/features/explore/Logs.tsx @@ -1,4 +1,4 @@ -import React, { PureComponent } from 'react'; +import React, { PureComponent, createRef } from 'react'; import { css } from '@emotion/css'; import { capitalize } from 'lodash'; import memoizeOne from 'memoize-one'; @@ -29,7 +29,6 @@ import { InlineSwitch, withTheme, stylesFactory, - CustomScrollbar, } from '@grafana/ui'; import store from 'app/core/store'; import { dedupLogRows, filterLogLevels } from 'app/core/logs_model'; @@ -83,6 +82,7 @@ interface State { export class UnthemedLogs extends PureComponent { flipOrderTimer: NodeJS.Timeout; cancelFlippingTimer: NodeJS.Timeout; + topLogsRef = createRef(); state: State = { showLabels: store.getBool(SETTINGS_KEYS.showLabels, false), @@ -222,6 +222,8 @@ export class UnthemedLogs extends PureComponent { return filterLogLevels(logRows, new Set(hiddenLogLevels)); }); + scrollToTopLogs = () => this.topLogsRef.current?.scrollIntoView(); + render() { const { logRows, @@ -280,7 +282,7 @@ export class UnthemedLogs extends PureComponent { showLines={false} onUpdateTimeRange={onChangeTime} /> -
+
@@ -327,7 +329,7 @@ export class UnthemedLogs extends PureComponent { clearDetectedFields={this.clearDetectedFields} />
- +
{ onClickShowDetectedField={this.showDetectedField} onClickHideDetectedField={this.hideDetectedField} /> - +
{ onChangeTime={onChangeTime} loading={loading} queries={queries} + scrollToTopLogs={this.scrollToTopLogs} />
{!loading && !hasData && !scanning && ( @@ -410,7 +413,10 @@ const getStyles = stylesFactory((theme: GrafanaTheme) => { logsSection: css` display: flex; flex-direction: row; - max-height: 95vh; + justify-content: space-between; + `, + logRows: css` + overflow-x: scroll; `, }; }); diff --git a/public/app/features/explore/LogsContainer.tsx b/public/app/features/explore/LogsContainer.tsx index f84d44e381f..f6cc9ac2f50 100644 --- a/public/app/features/explore/LogsContainer.tsx +++ b/public/app/features/explore/LogsContainer.tsx @@ -1,13 +1,11 @@ import React, { PureComponent } from 'react'; import { hot } from 'react-hot-loader'; import { connect, ConnectedProps } from 'react-redux'; +import { css } from 'emotion'; import { Collapse } from '@grafana/ui'; - import { AbsoluteTimeRange, Field, LogRowModel, RawTimeRange } from '@grafana/data'; - import { ExploreId, ExploreItemState } from 'app/types/explore'; import { StoreState } from 'app/types'; - import { splitOpen } from './state/main'; import { updateTimeRange } from './state/time'; import { getTimeZone } from '../profile/state/selectors'; @@ -85,6 +83,16 @@ export class LogsContainer extends PureComponent div { + overflow: visible; + & > div { + overflow: visible; + } + } + `; + return ( <> @@ -104,7 +112,7 @@ export class LogsContainer extends PureComponent - + ; +type LogsNavigationProps = ComponentProps; const setup = (propOverrides?: object) => { - const props: LogsNavigationrProps = { + const props: LogsNavigationProps = { absoluteRange: { from: 1619081645930, to: 1619081945930 }, timeZone: 'local', queries: [], @@ -14,6 +14,7 @@ const setup = (propOverrides?: object) => { logsSortOrder: undefined, visibleRange: { from: 1619081941000, to: 1619081945930 }, onChangeTime: jest.fn(), + scrollToTopLogs: jest.fn(), ...propOverrides, }; @@ -21,24 +22,41 @@ const setup = (propOverrides?: object) => { }; describe('LogsNavigation', () => { - it('should render fetch logs button on bottom when default logs order', () => { + it('should always render 3 navigation buttons', () => { setup(); - expect(screen.getByTestId('fetchLogsBottom')).toBeInTheDocument(); - expect(screen.queryByTestId('fetchLogsTop')).not.toBeInTheDocument(); + expect(screen.getByTestId('newerLogsButton')).toBeInTheDocument(); + expect(screen.getByTestId('olderLogsButton')).toBeInTheDocument(); + expect(screen.getByTestId('scrollToTop')).toBeInTheDocument(); }); - it('should render fetch logs button on top when flipped logs order', () => { - setup({ logsSortOrder: LogsSortOrder.Ascending }); - expect(screen.getByTestId('fetchLogsTop')).toBeInTheDocument(); - expect(screen.queryByTestId('fetchLogsBottom')).not.toBeInTheDocument(); + + it('should render 3 navigation buttons in correct order when default logs order', () => { + const { container } = setup(); + const expectedOrder = ['newerLogsButton', 'olderLogsButton', 'scrollToTop']; + const elements = container.querySelectorAll( + '[data-testid=newerLogsButton],[data-testid=olderLogsButton],[data-testid=scrollToTop]' + ); + expect(Array.from(elements).map((el) => el.getAttribute('data-testid'))).toMatchObject(expectedOrder); }); - it('should disable button to fetch logs when loading', () => { + + it('should render 3 navigation buttons in correect order when flipped logs order', () => { + const { container } = setup({ logsSortOrder: LogsSortOrder.Ascending }); + const expectedOrder = ['olderLogsButton', 'newerLogsButton', 'scrollToTop']; + const elements = container.querySelectorAll( + '[data-testid=newerLogsButton],[data-testid=olderLogsButton],[data-testid=scrollToTop]' + ); + expect(Array.from(elements).map((el) => el.getAttribute('data-testid'))).toMatchObject(expectedOrder); + }); + + it('should disable fetch buttons when logs are loading', () => { setup({ loading: true }); - const button = screen.getByTestId('fetchLogsBottom'); - expect(button).toBeInTheDocument(); - expect(button).toBeDisabled(); + const olderLogsButton = screen.getByTestId('olderLogsButton'); + const newerLogsButton = screen.getByTestId('newerLogsButton'); + expect(olderLogsButton).toBeDisabled(); + expect(newerLogsButton).toBeDisabled(); }); - it('should render logs page with correct range', () => { + + it('should render logs navigation pages section', () => { setup(); - expect(screen.getByText(/02:59:05 — 02:59:01/i)).toBeInTheDocument(); + expect(screen.getByTestId('logsNavigationPages')).toBeInTheDocument(); }); }); diff --git a/public/app/features/explore/LogsNavigation.tsx b/public/app/features/explore/LogsNavigation.tsx index 56b436d4783..9b4ae51dc24 100644 --- a/public/app/features/explore/LogsNavigation.tsx +++ b/public/app/features/explore/LogsNavigation.tsx @@ -1,17 +1,9 @@ import React, { memo, useState, useEffect, useRef } from 'react'; -import classNames from 'classnames'; import { isEqual } from 'lodash'; import { css } from 'emotion'; -import { - LogsSortOrder, - AbsoluteTimeRange, - dateTimeFormat, - systemDateFormats, - TimeZone, - DataQuery, - GrafanaTheme, -} from '@grafana/data'; -import { Button, Icon, Spinner, useTheme, stylesFactory, CustomScrollbar } from '@grafana/ui'; +import { LogsSortOrder, AbsoluteTimeRange, TimeZone, DataQuery, GrafanaTheme } from '@grafana/data'; +import { Button, Icon, Spinner, useTheme, stylesFactory } from '@grafana/ui'; +import { LogsNavigationPages } from './LogsNavigationPages'; type Props = { absoluteRange: AbsoluteTimeRange; @@ -21,9 +13,10 @@ type Props = { visibleRange?: AbsoluteTimeRange; logsSortOrder?: LogsSortOrder | null; onChangeTime: (range: AbsoluteTimeRange) => void; + scrollToTopLogs: () => void; }; -type LogsPage = { +export type LogsPage = { logsRange: AbsoluteTimeRange; queryRange: AbsoluteTimeRange; }; @@ -34,6 +27,7 @@ function LogsNavigation({ timeZone, loading, onChangeTime, + scrollToTopLogs, visibleRange = absoluteRange, queries = [], }: Props) { @@ -47,6 +41,12 @@ function LogsNavigation({ // e.g. if last 5 min selected, always run 5 min range const rangeSpanRef = useRef(0); + const oldestLogsFirst = logsSortOrder === LogsSortOrder.Ascending; + const onFirstPage = currentPageIndex === 0; + const onLastPage = currentPageIndex === pages.length - 1; + const theme = useTheme(); + const styles = getStyles(theme, oldestLogsFirst, loading); + // Main effect to set pages and index useEffect(() => { const newPage = { logsRange: visibleRange, queryRange: absoluteRange }; @@ -86,85 +86,77 @@ function LogsNavigation({ return a.queryRange.to > b.queryRange.to ? -1 : 1; }; - const formatTime = (time: number) => { - return `${dateTimeFormat(time, { - format: systemDateFormats.interval.second, - timeZone: timeZone, - })}`; - }; + const olderLogsButton = ( + + ); - const createPageContent = (page: LogsPage, index: number) => { - if (currentPageIndex === index && loading) { - return ; - } - const topContent = formatTime(oldestLogsFirst ? page.logsRange.from : page.logsRange.to); - const bottomContent = formatTime(oldestLogsFirst ? page.logsRange.to : page.logsRange.from); - return `${topContent} — ${bottomContent}`; - }; + const newerLogsButton = ( + + ); - const oldestLogsFirst = logsSortOrder === LogsSortOrder.Ascending; - const theme = useTheme(); - const styles = getStyles(theme, oldestLogsFirst, loading); return (
- {/* - * We are going to have 2 buttons - on the top and bottom - Oldest and Newest. - * Therefore I have at the moment duplicated the same code, but in the future iteration, it ill be updated - */} - {oldestLogsFirst && ( - - )} - -
-
- {pages.map((page: LogsPage, index) => ( -
!loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to })} - > -
-
- {createPageContent(page, index)} -
-
- ))} -
-
-
- - - {!oldestLogsFirst && ( - - )} + {oldestLogsFirst ? olderLogsButton : newerLogsButton} + + {oldestLogsFirst ? newerLogsButton : olderLogsButton} +
); } @@ -178,6 +170,9 @@ const getStyles = stylesFactory((theme: GrafanaTheme, oldestLogsFirst: boolean, display: flex; flex-direction: column; justify-content: ${oldestLogsFirst ? 'flex-start' : 'space-between'}; + position: sticky; + top: ${theme.spacing.md}; + right: 0; `, navButton: css` width: 58px; @@ -197,55 +192,14 @@ const getStyles = stylesFactory((theme: GrafanaTheme, oldestLogsFirst: boolean, height: 100%; white-space: normal; `, - pagesWrapper: css` - height: 100%; - padding-left: ${theme.spacing.xs}; + scrollToTopButton: css` + width: 40px; + height: 40px; display: flex; flex-direction: column; - overflow-y: scroll; - `, - pagesContainer: css` - display: flex; - padding: 0; - flex-direction: column; - `, - page: css` - display: flex; - margin: ${theme.spacing.md} 0; - cursor: ${loading ? 'auto' : 'pointer'}; - white-space: normal; - .selectedBg { - background: ${theme.colors.bgBlue2}; - } - .selectedText { - color: ${theme.colors.bgBlue2}; - } - `, - line: css` - width: 3px; - height: 100%; + justify-content: center; align-items: center; - background: ${theme.colors.textWeak}; - `, - time: css` - width: 60px; - min-height: 80px; - font-size: ${theme.typography.size.sm}; - padding-left: ${theme.spacing.xs}; - display: flex; - align-items: center; - `, - filler: css` - height: inherit; - background: repeating-linear-gradient( - 135deg, - ${theme.colors.bg1}, - ${theme.colors.bg1} 5px, - ${theme.colors.bg2} 5px, - ${theme.colors.bg2} 15px - ); - width: 3px; - margin-bottom: 8px; + margin-top: ${theme.spacing.sm}; `, }; }); diff --git a/public/app/features/explore/LogsNavigationPages.test.tsx b/public/app/features/explore/LogsNavigationPages.test.tsx new file mode 100644 index 00000000000..9b1b7790326 --- /dev/null +++ b/public/app/features/explore/LogsNavigationPages.test.tsx @@ -0,0 +1,45 @@ +import React, { ComponentProps } from 'react'; +import { render, screen } from '@testing-library/react'; +import { LogsNavigationPages } from './LogsNavigationPages'; + +type LogsNavigationPagesProps = ComponentProps; + +const setup = (propOverrides?: object) => { + const props: LogsNavigationPagesProps = { + pages: [ + { + logsRange: { from: 1619081941000, to: 1619081945930 }, + queryRange: { from: 1619081645930, to: 1619081945930 }, + }, + { + logsRange: { from: 1619081951000, to: 1619081955930 }, + queryRange: { from: 1619081655930, to: 1619081955930 }, + }, + ], + currentPageIndex: 0, + oldestLogsFirst: false, + timeZone: 'local', + loading: false, + changeTime: jest.fn(), + ...propOverrides, + }; + + return render(); +}; + +describe('LogsNavigationPages', () => { + it('should render logs navigation pages', () => { + setup(); + expect(screen.getByTestId('logsNavigationPages')).toBeInTheDocument(); + }); + it('should render logs pages with correct range if normal order', () => { + setup(); + expect(screen.getByText(/02:59:05 — 02:59:01/i)).toBeInTheDocument(); + expect(screen.getByText(/02:59:15 — 02:59:11/i)).toBeInTheDocument(); + }); + it('should render logs pages with correct range if flipped order', () => { + setup({ oldestLogsFirst: true }); + expect(screen.getByText(/02:59:11 — 02:59:15/i)).toBeInTheDocument(); + expect(screen.getByText(/02:59:01 — 02:59:05/i)).toBeInTheDocument(); + }); +}); diff --git a/public/app/features/explore/LogsNavigationPages.tsx b/public/app/features/explore/LogsNavigationPages.tsx new file mode 100644 index 00000000000..4ead2bfd475 --- /dev/null +++ b/public/app/features/explore/LogsNavigationPages.tsx @@ -0,0 +1,121 @@ +import React from 'react'; +import { css, cx } from 'emotion'; +import { dateTimeFormat, systemDateFormats, TimeZone, GrafanaTheme, AbsoluteTimeRange } from '@grafana/data'; +import { useTheme, stylesFactory, CustomScrollbar, Spinner } from '@grafana/ui'; +import { LogsPage } from './LogsNavigation'; + +type Props = { + pages: LogsPage[]; + currentPageIndex: number; + oldestLogsFirst: boolean; + timeZone: TimeZone; + loading: boolean; + changeTime: (range: AbsoluteTimeRange) => void; +}; + +export function LogsNavigationPages({ + pages, + currentPageIndex, + oldestLogsFirst, + timeZone, + loading, + changeTime, +}: Props) { + const formatTime = (time: number) => { + return `${dateTimeFormat(time, { + format: systemDateFormats.interval.second, + timeZone: timeZone, + })}`; + }; + + const createPageContent = (page: LogsPage, index: number) => { + if (currentPageIndex === index && loading) { + return ; + } + const topContent = formatTime(oldestLogsFirst ? page.logsRange.from : page.logsRange.to); + const bottomContent = formatTime(oldestLogsFirst ? page.logsRange.to : page.logsRange.from); + return `${topContent} — ${bottomContent}`; + }; + + const theme = useTheme(); + const styles = getStyles(theme, loading); + + return ( + +
+
+ {pages.map((page: LogsPage, index: number) => ( +
!loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to })} + > +
+
+ {createPageContent(page, index)} +
+
+ ))} +
+
+ + ); +} + +const getStyles = stylesFactory((theme: GrafanaTheme, loading: boolean) => { + return { + pagesWrapper: css` + height: 100%; + padding-left: ${theme.spacing.xs}; + display: flex; + flex-direction: column; + overflow-y: scroll; + &::after { + content: ''; + display: block; + background: repeating-linear-gradient( + 135deg, + ${theme.colors.bg1}, + ${theme.colors.bg1} 5px, + ${theme.colors.bg2} 5px, + ${theme.colors.bg2} 15px + ); + width: 3px; + height: inherit; + margin-bottom: 8px; + } + `, + pagesContainer: css` + display: flex; + padding: 0; + flex-direction: column; + `, + page: css` + display: flex; + margin: ${theme.spacing.md} 0; + cursor: ${loading ? 'auto' : 'pointer'}; + white-space: normal; + .selectedBg { + background: ${theme.colors.bgBlue2}; + } + .selectedText { + color: ${theme.colors.bgBlue2}; + } + `, + line: css` + width: 3px; + height: 100%; + align-items: center; + background: ${theme.colors.textWeak}; + `, + time: css` + width: 60px; + min-height: 80px; + font-size: ${theme.typography.size.sm}; + padding-left: ${theme.spacing.xs}; + display: flex; + align-items: center; + `, + }; +});