From 409bd33a8f9b79df4873504cf6b0f496665e227f Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Thu, 23 Mar 2023 15:17:33 +0100 Subject: [PATCH] Alerting: Paginate result previews (#65257) Co-authored-by: konrad147 Co-authored-by: Sonia Aguilar --- .../expressions/Expression.test.tsx | 66 ++++++++++++++++++ .../components/expressions/Expression.tsx | 68 ++++++++++++++++--- .../unified/hooks/usePagination.test.tsx | 67 ++++++++++++++++++ .../alerting/unified/hooks/usePagination.ts | 26 ++++--- 4 files changed, 206 insertions(+), 21 deletions(-) create mode 100644 public/app/features/alerting/unified/components/expressions/Expression.test.tsx create mode 100644 public/app/features/alerting/unified/hooks/usePagination.test.tsx diff --git a/public/app/features/alerting/unified/components/expressions/Expression.test.tsx b/public/app/features/alerting/unified/components/expressions/Expression.test.tsx new file mode 100644 index 00000000000..7c4e2746940 --- /dev/null +++ b/public/app/features/alerting/unified/components/expressions/Expression.test.tsx @@ -0,0 +1,66 @@ +import { screen, render } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { times } from 'lodash'; +import React from 'react'; + +import { DataFrame, toDataFrame } from '@grafana/data'; + +import { ExpressionResult } from './Expression'; + +describe('TestResult', () => { + it('should be able to render', () => { + expect(() => { + render(); + }).not.toThrow(); + }); + + it('should not paginate with less than PAGE_SIZE', () => { + const series: DataFrame[] = [ + toDataFrame({ + fields: [ + { + name: 'temp', + values: [23, 11, 10], + }, + ], + }), + ]; + + render(); + expect(screen.queryByTestId('paginate-expression')).not.toBeInTheDocument(); + }); + + it('should paginate with greater than PAGE_SIZE', async () => { + const series: DataFrame[] = makeSeries(50); + + render(); + expect(screen.getByTestId('paginate-expression')).toBeInTheDocument(); + expect(screen.getByText(`1 - 20 of ${50}`)).toBeInTheDocument(); + + // click previous page + await userEvent.click(screen.getByLabelText('previous-page')); + expect(screen.getByText(`1 - 20 of ${50}`)).toBeInTheDocument(); + + // keep clicking next page, should clamp + await userEvent.click(screen.getByLabelText('next-page')); + expect(screen.getByText(`21 - 40 of ${50}`)).toBeInTheDocument(); + await userEvent.click(screen.getByLabelText('next-page')); + expect(screen.getByText(`41 - 50 of ${50}`)).toBeInTheDocument(); + // click one more time, should still be on the last page + await userEvent.click(screen.getByLabelText('next-page')); + expect(screen.getByText(`41 - 50 of ${50}`)).toBeInTheDocument(); + }); +}); + +function makeSeries(n: number) { + return times(n, () => + toDataFrame({ + fields: [ + { + name: 'temp', + values: [1], + }, + ], + }) + ); +} diff --git a/public/app/features/alerting/unified/components/expressions/Expression.tsx b/public/app/features/alerting/unified/components/expressions/Expression.tsx index 7504a8e5e70..e5ae64df1e9 100644 --- a/public/app/features/alerting/unified/components/expressions/Expression.tsx +++ b/public/app/features/alerting/unified/components/expressions/Expression.tsx @@ -2,9 +2,9 @@ import { css, cx } from '@emotion/css'; import { capitalize, uniqueId } from 'lodash'; import React, { FC, useCallback, useState } from 'react'; -import { DataFrame, dateTimeFormat, GrafanaTheme2, LoadingState, PanelData, isTimeSeriesFrames } from '@grafana/data'; +import { DataFrame, dateTimeFormat, GrafanaTheme2, isTimeSeriesFrames, LoadingState, PanelData } from '@grafana/data'; import { Stack } from '@grafana/experimental'; -import { AutoSizeInput, clearButtonStyles, Icon, IconButton, Select, useStyles2 } from '@grafana/ui'; +import { AutoSizeInput, Button, clearButtonStyles, Icon, IconButton, Select, useStyles2 } from '@grafana/ui'; import { ClassicConditions } from 'app/features/expressions/components/ClassicConditions'; import { Math } from 'app/features/expressions/components/Math'; import { Reduce } from 'app/features/expressions/components/Reduce'; @@ -13,6 +13,7 @@ import { Threshold } from 'app/features/expressions/components/Threshold'; import { ExpressionQuery, ExpressionQueryType, gelTypes } from 'app/features/expressions/types'; import { AlertQuery, PromAlertingRuleState } from 'app/types/unified-alerting-dto'; +import { usePagination } from '../../hooks/usePagination'; import { HoverCard } from '../HoverCard'; import { Spacer } from '../Spacer'; import { AlertStateTag } from '../rules/AlertStateTag'; @@ -105,6 +106,7 @@ export const Expression: FC = ({ />
{renderExpressionType(query)}
{hasResults && } +
= ({ series, isAlertCondition }) => { + const { page, pageItems, onPageChange, numberOfPages, pageStart, pageEnd } = usePagination(series, 1, PAGE_SIZE); const styles = useStyles2(getStyles); // sometimes we receive results where every value is just "null" when noData occurs const emptyResults = isEmptySeries(series); const isTimeSeriesResults = !emptyResults && isTimeSeriesFrames(series); + const previousPage = useCallback(() => { + onPageChange(page - 1); + }, [page, onPageChange]); + + const nextPage = useCallback(() => { + onPageChange(page + 1); + }, [page, onPageChange]); + + const shouldShowPagination = numberOfPages > 1; + return (
{!emptyResults && isTimeSeriesResults && (
- {series.map((frame, index) => ( - + {pageItems.map((frame, index) => ( + ))}
)} {!emptyResults && !isTimeSeriesResults && - series.map((frame, index) => ( + pageItems.map((frame, index) => ( // There's no way to uniquely identify a frame that doesn't cause render bugs :/ (Gilles) - + ))} {emptyResults &&
No data
} + {shouldShowPagination && ( +
+ +
+ )}
); }; @@ -429,11 +474,8 @@ const getStyles = (theme: GrafanaTheme2) => ({ `, timeseriesTableWrapper: css` max-height: 500px; - max-width: 300px; overflow-y: scroll; - - padding: 0 !important; // not sure why but style override doesn't work otherwise :( (Gilles) `, timeseriesTable: css` table-layout: auto; @@ -462,4 +504,10 @@ const getStyles = (theme: GrafanaTheme2) => ({ } } `, + pagination: { + wrapper: css` + border-top: 1px solid ${theme.colors.border.medium}; + padding: ${theme.spacing()}; + `, + }, }); diff --git a/public/app/features/alerting/unified/hooks/usePagination.test.tsx b/public/app/features/alerting/unified/hooks/usePagination.test.tsx new file mode 100644 index 00000000000..48793b0ec5c --- /dev/null +++ b/public/app/features/alerting/unified/hooks/usePagination.test.tsx @@ -0,0 +1,67 @@ +import { act, renderHook } from '@testing-library/react-hooks'; + +import { usePagination } from './usePagination'; + +describe('usePagination()', () => { + it('should work with no items', () => { + const { result } = renderHook(() => { + return usePagination([], 1, 20); + }); + + const { pageItems, numberOfPages, page, pageStart, pageEnd } = result.current; + + expect(pageItems).toStrictEqual([]); + expect(numberOfPages).toStrictEqual(0); + expect(page).toStrictEqual(1); + expect(pageStart).toStrictEqual(1); + expect(pageEnd).toStrictEqual(0); + }); + + it('should work with items < page size', () => { + const { result } = renderHook(() => { + return usePagination([1, 2, 3], 1, 10); + }); + + const { pageItems, numberOfPages, page, pageStart, pageEnd } = result.current; + + expect(pageItems).toStrictEqual([1, 2, 3]); + expect(numberOfPages).toStrictEqual(1); + expect(page).toStrictEqual(1); + expect(pageStart).toStrictEqual(1); + expect(pageEnd).toStrictEqual(3); + }); + + it('should work with items > page size', () => { + const { result } = renderHook(() => { + return usePagination([1, 2, 3], 1, 1); + }); + + const { pageItems, numberOfPages, page, pageStart, pageEnd } = result.current; + + expect(pageItems).toStrictEqual([1]); + expect(numberOfPages).toStrictEqual(3); + expect(page).toStrictEqual(1); + expect(pageStart).toStrictEqual(1); + expect(pageEnd).toStrictEqual(1); + }); + + it('should clamp pages', () => { + const { result } = renderHook(() => { + return usePagination([1, 2, 3], 1, 1); + }); + + expect(result.current.pageItems).toStrictEqual([1]); + + act(() => result.current.previousPage()); + expect(result.current.pageItems).toStrictEqual([1]); + + act(() => result.current.nextPage()); + expect(result.current.pageItems).toStrictEqual([2]); + + act(() => result.current.nextPage()); + expect(result.current.pageItems).toStrictEqual([3]); + + act(() => result.current.nextPage()); + expect(result.current.pageItems).toStrictEqual([3]); + }); +}); diff --git a/public/app/features/alerting/unified/hooks/usePagination.ts b/public/app/features/alerting/unified/hooks/usePagination.ts index 1c7b1bfae6b..e95962d9a7a 100644 --- a/public/app/features/alerting/unified/hooks/usePagination.ts +++ b/public/app/features/alerting/unified/hooks/usePagination.ts @@ -1,25 +1,29 @@ -import { useCallback, useEffect, useMemo, useState } from 'react'; +import { chunk, clamp } from 'lodash'; +import { useCallback, useEffect, useState, useMemo } from 'react'; -export function usePagination(items: T[], initialPage: number, itemsPerPage: number) { +export function usePagination(items: T[], initialPage = 1, itemsPerPage: number) { const [page, setPage] = useState(initialPage); - const numberOfPages = Math.ceil(items.length / itemsPerPage); - const firstItemOnPageIndex = itemsPerPage * (page - 1); + const pages = useMemo(() => chunk(items, itemsPerPage), [items, itemsPerPage]); - const pageItems = useMemo( - () => items.slice(firstItemOnPageIndex, firstItemOnPageIndex + itemsPerPage), - [items, firstItemOnPageIndex, itemsPerPage] - ); + const numberOfPages = pages.length; + const pageItems = pages[page - 1] ?? []; + + const pageStart = (page - 1) * itemsPerPage + 1; + const pageEnd = clamp(page * itemsPerPage, items.length); const onPageChange = useCallback( (newPage: number) => { - setPage(newPage); + setPage(clamp(newPage, 1, pages.length)); }, - [setPage] + [setPage, pages] ); + const nextPage = useCallback(() => onPageChange(page + 1), [page, onPageChange]); + const previousPage = useCallback(() => onPageChange(page - 1), [page, onPageChange]); + // Reset the current page when number of pages has been changed useEffect(() => setPage(1), [numberOfPages]); - return { page, onPageChange, numberOfPages, pageItems }; + return { page, onPageChange, numberOfPages, pageItems, pageStart, pageEnd, nextPage, previousPage }; }