Explore: Polish UI of logs navigation (#33910)

* 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 <gabor.farkas@gmail.com>

* Update public/app/features/explore/LogsNavigation.test.tsx

Co-authored-by: Gábor Farkas <gabor.farkas@gmail.com>

* Remove unnecessary memoization

* Update public/app/features/explore/LogsNavigationPages.tsx

Co-authored-by: Gábor Farkas <gabor.farkas@gmail.com>

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;

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<Props, State> {
   flipOrderTimer: NodeJS.Timeout;
   cancelFlippingTimer: NodeJS.Timeout;
+  topLogsRef = createRef<HTMLDivElement>();
 
   state: State = {
     showLabels: store.getBool(SETTINGS_KEYS.showLabels, false),
@@ -222,6 +222,8 @@ export class UnthemedLogs extends PureComponent<Props, State> {
     return filterLogLevels(logRows, new Set(hiddenLogLevels));
   });
 
+  scrollToTopLogs = () => this.topLogsRef.current?.scrollIntoView();
+
   render() {
     const {
       logRows,
@@ -280,7 +282,7 @@ export class UnthemedLogs extends PureComponent<Props, State> {
           showLines={false}
           onUpdateTimeRange={onChangeTime}
         />
-        <div className={styles.logOptions}>
+        <div className={styles.logOptions} ref={this.topLogsRef}>
           <InlineFieldRow>
             <InlineField label="Time" transparent>
               <InlineSwitch value={showTime} onChange={this.onChangeTime} transparent />
@@ -327,7 +329,7 @@ export class UnthemedLogs extends PureComponent<Props, State> {
           clearDetectedFields={this.clearDetectedFields}
         />
         <div className={styles.logsSection}>
-          <CustomScrollbar autoHide>
+          <div className={styles.logRows}>
             <LogRows
               logRows={logRows}
               deduplicatedRows={dedupedRows}
@@ -348,7 +350,7 @@ export class UnthemedLogs extends PureComponent<Props, State> {
               onClickShowDetectedField={this.showDetectedField}
               onClickHideDetectedField={this.hideDetectedField}
             />
-          </CustomScrollbar>
+          </div>
           <LogsNavigation
             logsSortOrder={logsSortOrder}
             visibleRange={visibleRange}
@@ -357,6 +359,7 @@ export class UnthemedLogs extends PureComponent<Props, State> {
             onChangeTime={onChangeTime}
             loading={loading}
             queries={queries}
+            scrollToTopLogs={this.scrollToTopLogs}
           />
         </div>
         {!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;
     `,
   };
 });

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<PropsFromRedux & LogsContainerP
       return null;
     }
 
+    // We need to override css overflow of divs in Collapse element to enable sticky Logs navigation
+    const styleOverridesForStickyNavigation = css`
+      & > div {
+        overflow: visible;
+        & > div {
+          overflow: visible;
+        }
+      }
+    `;
+
     return (
       <>
         <LogsCrossFadeTransition visible={isLive}>
@@ -104,7 +112,7 @@ export class LogsContainer extends PureComponent<PropsFromRedux & LogsContainerP
           </Collapse>
         </LogsCrossFadeTransition>
         <LogsCrossFadeTransition visible={!isLive}>
-          <Collapse label="Logs" loading={loading} isOpen>
+          <Collapse label="Logs" loading={loading} isOpen className={styleOverridesForStickyNavigation}>
             <Logs
               logRows={logRows}
               logsMeta={logsMeta}

public/app/features/explore/LogsNavigation.test.tsx

@@ -3,10 +3,10 @@ import { render, screen } from '@testing-library/react';
 import { LogsSortOrder } from '@grafana/data';
 import LogsNavigation from './LogsNavigation';
 
-type LogsNavigationrProps = ComponentProps<typeof LogsNavigation>;
+type LogsNavigationProps = ComponentProps<typeof LogsNavigation>;
 
 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.getByTestId('newerLogsButton')).toBeInTheDocument();
-    expect(screen.queryByTestId('fetchLogsTop')).not.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 });
+  it('should render 3 navigation buttons in correct order when default logs order', () => {
-    expect(screen.getByTestId('fetchLogsTop')).toBeInTheDocument();
+    const { container } = setup();
-    expect(screen.queryByTestId('fetchLogsBottom')).not.toBeInTheDocument();
+    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');
+    const olderLogsButton = screen.getByTestId('olderLogsButton');
-    expect(button).toBeInTheDocument();
+    const newerLogsButton = screen.getByTestId('newerLogsButton');
-    expect(button).toBeDisabled();
+    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();
   });
 });

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 {
+import { LogsSortOrder, AbsoluteTimeRange, TimeZone, DataQuery, GrafanaTheme } from '@grafana/data';
-  LogsSortOrder,
+import { Button, Icon, Spinner, useTheme, stylesFactory } from '@grafana/ui';
-  AbsoluteTimeRange,
+import { LogsNavigationPages } from './LogsNavigationPages';
-  dateTimeFormat,
-  systemDateFormats,
-  TimeZone,
-  DataQuery,
-  GrafanaTheme,
-} from '@grafana/data';
-import { Button, Icon, Spinner, useTheme, stylesFactory, CustomScrollbar } from '@grafana/ui';
 
 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) => {
+  const olderLogsButton = (
-    return `${dateTimeFormat(time, {
+    <Button
-      format: systemDateFormats.interval.second,
+      data-testid="olderLogsButton"
-      timeZone: timeZone,
+      className={styles.navButton}
-    })}`;
+      variant="secondary"
-  };
+      onClick={() => {
-
+        //If we are not on the last page, use next page's range
-  const createPageContent = (page: LogsPage, index: number) => {
+        if (!onLastPage) {
-    if (currentPageIndex === index && loading) {
+          changeTime({
-      return <Spinner />;
+            from: pages[currentPageIndex + 1].queryRange.from,
+            to: pages[currentPageIndex + 1].queryRange.to,
+          });
         }
-    const topContent = formatTime(oldestLogsFirst ? page.logsRange.from : page.logsRange.to);
+        //If we are on the last page, create new range
-    const bottomContent = formatTime(oldestLogsFirst ? page.logsRange.to : page.logsRange.from);
+        changeTime({ from: visibleRange.from - rangeSpanRef.current, to: visibleRange.from });
-    return `${topContent} — ${bottomContent}`;
+      }}
-  };
+      disabled={loading}
+    >
+      <div className={styles.navButtonContent}>
+        {loading ? <Spinner /> : <Icon name={oldestLogsFirst ? 'angle-up' : 'angle-down'} size="lg" />}
+        Older logs
+      </div>
+    </Button>
+  );
+
+  const newerLogsButton = (
+    <Button
+      data-testid="newerLogsButton"
+      className={styles.navButton}
+      variant="secondary"
+      onClick={() => {
+        //If we are not on the first page, use previous page's range
+        if (!onFirstPage) {
+          changeTime({
+            from: pages[currentPageIndex - 1].queryRange.from,
+            to: pages[currentPageIndex - 1].queryRange.to,
+          });
+        }
+        //If we are on the first page, button is disabled and we do nothing
+      }}
+      disabled={loading || onFirstPage}
+    >
+      <div className={styles.navButtonContent}>
+        {loading && <Spinner />}
+        {onFirstPage || loading ? null : <Icon name={oldestLogsFirst ? 'angle-down' : 'angle-up'} size="lg" />}
+        {onFirstPage ? 'Start of range' : 'Newer logs'}
+      </div>
+    </Button>
+  );
 
-  const oldestLogsFirst = logsSortOrder === LogsSortOrder.Ascending;
-  const theme = useTheme();
-  const styles = getStyles(theme, oldestLogsFirst, loading);
   return (
     <div className={styles.navContainer}>
-      {/*
+      {oldestLogsFirst ? olderLogsButton : newerLogsButton}
-       * We are going to have 2 buttons - on the top and bottom - Oldest and Newest.
+      <LogsNavigationPages
-       * Therefore I have at the moment duplicated the same code, but in the future iteration, it ill be updated
+        pages={pages}
-       */}
+        currentPageIndex={currentPageIndex}
-      {oldestLogsFirst && (
+        oldestLogsFirst={oldestLogsFirst}
+        timeZone={timeZone}
+        loading={loading}
+        changeTime={changeTime}
+      />
+      {oldestLogsFirst ? newerLogsButton : olderLogsButton}
       <Button
-          data-testid="fetchLogsTop"
+        data-testid="scrollToTop"
-          className={styles.navButton}
+        className={styles.scrollToTopButton}
         variant="secondary"
-          onClick={() => {
+        onClick={scrollToTopLogs}
-            // the range is based on initally selected range
+        title="Scroll to top"
-            changeTime({ from: visibleRange.from - rangeSpanRef.current, to: visibleRange.from });
-          }}
-          disabled={loading}
       >
-          <div className={styles.navButtonContent}>
+        <Icon name="arrow-up" size="lg" />
-            {loading ? <Spinner /> : <Icon name="angle-up" size="lg" />}
-            Older logs
-          </div>
       </Button>
-      )}
-      <CustomScrollbar autoHide>
-        <div className={styles.pagesWrapper}>
-          <div className={styles.pagesContainer}>
-            {pages.map((page: LogsPage, index) => (
-              <div
-                className={styles.page}
-                key={page.queryRange.to}
-                onClick={() => !loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to })}
-              >
-                <div className={classNames(styles.line, { selectedBg: currentPageIndex === index })} />
-                <div className={classNames(styles.time, { selectedText: currentPageIndex === index })}>
-                  {createPageContent(page, index)}
-                </div>
-              </div>
-            ))}
-          </div>
-          <div className={styles.filler}></div>
-        </div>
-      </CustomScrollbar>
-
-      {!oldestLogsFirst && (
-        <Button
-          data-testid="fetchLogsBottom"
-          className={styles.navButton}
-          variant="secondary"
-          onClick={() => {
-            // the range is based on initally selected range
-            changeTime({ from: visibleRange.from - rangeSpanRef.current, to: visibleRange.from });
-          }}
-          disabled={loading}
-        >
-          <div className={styles.navButtonContent}>
-            Older logs
-            {loading ? <Spinner /> : <Icon name="angle-down" size="lg" />}
-          </div>
-        </Button>
-      )}
     </div>
   );
 }
@@ -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`
+    scrollToTopButton: css`
-      height: 100%;
+      width: 40px;
-      padding-left: ${theme.spacing.xs};
+      height: 40px;
       display: flex;
       flex-direction: column;
-      overflow-y: scroll;
+      justify-content: center;
-    `,
-    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};
+      margin-top: ${theme.spacing.sm};
-    `,
-    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;
     `,
   };
 });

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<typeof LogsNavigationPages>;
+
+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(<LogsNavigationPages {...props} />);
+};
+
+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();
+  });
+});

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 <Spinner />;
+    }
+    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 (
+    <CustomScrollbar autoHide>
+      <div className={styles.pagesWrapper} data-testid="logsNavigationPages">
+        <div className={styles.pagesContainer}>
+          {pages.map((page: LogsPage, index: number) => (
+            <div
+              data-testid={`page${index + 1}`}
+              className={styles.page}
+              key={page.queryRange.to}
+              onClick={() => !loading && changeTime({ from: page.queryRange.from, to: page.queryRange.to })}
+            >
+              <div className={cx(styles.line, { selectedBg: currentPageIndex === index })} />
+              <div className={cx(styles.time, { selectedText: currentPageIndex === index })}>
+                {createPageContent(page, index)}
+              </div>
+            </div>
+          ))}
+        </div>
+      </div>
+    </CustomScrollbar>
+  );
+}
+
+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;
+    `,
+  };
+});