[ResponseOps] [RAM] Align flyout pagination with discover (#131193)

* Initial version * Uncomment for now * PR feedback Co-authored-by: Kibana Machine <[email protected]>
elastic · May 5, 2022 · 5ab0fa5 · 5ab0fa5
1 parent 3545f31
commit 5ab0fa5
Show file tree

Hide file tree

Showing 7 changed files with 183 additions and 127 deletions.
diff --git a/..._actions_ui/public/application/sections/alerts_table/alerts_flyout/alerts_flyout.test.tsx b/..._actions_ui/public/application/sections/alerts_table/alerts_flyout/alerts_flyout.test.tsx
@@ -11,16 +11,17 @@ import { AlertsFlyout } from './alerts_flyout';
 import { AlertsField } from '../../../../types';
 
 const onClose = jest.fn();
-const onPaginateNext = jest.fn();
-const onPaginatePrevious = jest.fn();
+const onPaginate = jest.fn();
 const props = {
   alert: {
     [AlertsField.name]: ['one'],
     [AlertsField.reason]: ['two'],
   },
+  flyoutIndex: 0,
+  alertsCount: 4,
+  isLoading: false,
   onClose,
-  onPaginateNext,
-  onPaginatePrevious,
+  onPaginate,
 };
 
 describe('AlertsFlyout', () => {
@@ -34,19 +35,31 @@ describe('AlertsFlyout', () => {
       await nextTick();
       wrapper.update();
     });
-    expect(wrapper.find('[data-test-subj="alertsFlyoutTitle"]').first().text()).toBe('one');
+    expect(wrapper.find('[data-test-subj="alertsFlyoutName"]').first().text()).toBe('one');
     expect(wrapper.find('[data-test-subj="alertsFlyoutReason"]').first().text()).toBe('two');
   });
 
-  it('should allow pagination', async () => {
+  it('should allow pagination with next', async () => {
     const wrapper = mountWithIntl(<AlertsFlyout {...props} />);
     await act(async () => {
       await nextTick();
       wrapper.update();
     });
-    wrapper.find('[data-test-subj="alertsFlyoutPaginatePrevious"]').first().simulate('click');
-    expect(onPaginatePrevious).toHaveBeenCalled();
-    wrapper.find('[data-test-subj="alertsFlyoutPaginateNext"]').first().simulate('click');
-    expect(onPaginateNext).toHaveBeenCalled();
+    wrapper.find('[data-test-subj="pagination-button-next"]').first().simulate('click');
+    expect(onPaginate).toHaveBeenCalledWith(1);
+  });
+
+  it('should allow pagination with previous', async () => {
+    const customProps = {
+      ...props,
+      flyoutIndex: 1,
+    };
+    const wrapper = mountWithIntl(<AlertsFlyout {...customProps} />);
+    await act(async () => {
+      await nextTick();
+      wrapper.update();
+    });
+    wrapper.find('[data-test-subj="pagination-button-previous"]').first().simulate('click');
+    expect(onPaginate).toHaveBeenCalledWith(0);
   });
 });
diff --git a/...ggers_actions_ui/public/application/sections/alerts_table/alerts_flyout/alerts_flyout.tsx b/...ggers_actions_ui/public/application/sections/alerts_table/alerts_flyout/alerts_flyout.tsx
@@ -15,81 +15,111 @@ import {
   EuiTitle,
   EuiText,
   EuiHorizontalRule,
-  EuiFlyoutFooter,
   EuiFlexGroup,
   EuiFlexItem,
-  EuiButton,
+  EuiPagination,
+  EuiProgress,
+  EuiLoadingContent,
 } from '@elastic/eui';
 import { AlertsField, AlertsData } from '../../../../types';
 
-const REASON_LABEL = i18n.translate(
-  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.reason',
+const SAMPLE_TITLE_LABEL = i18n.translate(
+  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.sampleTitle',
   {
-    defaultMessage: 'Reason',
+    defaultMessage: 'Sample title',
   }
 );
 
-const NEXT_LABEL = i18n.translate(
-  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.next',
+const NAME_LABEL = i18n.translate(
+  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.name',
   {
-    defaultMessage: 'Next',
+    defaultMessage: 'Name',
   }
 );
-const PREVIOUS_LABEL = i18n.translate(
-  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.previous',
+
+const REASON_LABEL = i18n.translate(
+  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.reason',
+  {
+    defaultMessage: 'Reason',
+  }
+);
+
+const PAGINATION_LABEL = i18n.translate(
+  'xpack.triggersActionsUI.sections.alertsTable.alertsFlyout.paginationLabel',
   {
-    defaultMessage: 'Previous',
+    defaultMessage: 'Alert navigation',
   }
 );
 
 interface AlertsFlyoutProps {
   alert: AlertsData;
+  flyoutIndex: number;
+  alertsCount: number;
+  isLoading: boolean;
   onClose: () => void;
-  onPaginateNext: () => void;
-  onPaginatePrevious: () => void;
+  onPaginate: (pageIndex: number) => void;
 }
 export const AlertsFlyout: React.FunctionComponent<AlertsFlyoutProps> = ({
   alert,
+  flyoutIndex,
+  alertsCount,
+  isLoading,
   onClose,
-  onPaginateNext,
-  onPaginatePrevious,
+  onPaginate,
 }: AlertsFlyoutProps) => {
   return (
     <EuiFlyout onClose={onClose} size="s" data-test-subj="alertsFlyout">
+      {isLoading && <EuiProgress size="xs" color="accent" data-test-subj="alertsFlyoutLoading" />}
       <EuiFlyoutHeader hasBorder>
-        <EuiTitle size="m" data-test-subj="alertsFlyoutTitle">
-          <h2>{get(alert, AlertsField.name)}</h2>
+        <EuiTitle size="m">
+          <h2>{SAMPLE_TITLE_LABEL}</h2>
         </EuiTitle>
+        <EuiSpacer size="m" />
+        <EuiFlexGroup gutterSize="none" justifyContent="flexEnd">
+          <EuiFlexItem grow={false}>
+            <EuiPagination
+              aria-label={PAGINATION_LABEL}
+              pageCount={alertsCount}
+              activePage={flyoutIndex}
+              onPageClick={onPaginate}
+              compressed
+              data-test-subj="alertsFlyoutPagination"
+            />
+          </EuiFlexItem>
+        </EuiFlexGroup>
       </EuiFlyoutHeader>
       <EuiFlyoutBody>
-        <EuiTitle size="xs">
-          <h4>{REASON_LABEL}</h4>
-        </EuiTitle>
-        <EuiSpacer size="s" />
-        <EuiText size="s" data-test-subj="alertsFlyoutReason">
-          {get(alert, AlertsField.reason)}
-        </EuiText>
-        <EuiSpacer size="s" />
-        <EuiHorizontalRule size="full" />
-      </EuiFlyoutBody>
-      <EuiFlyoutFooter>
-        <EuiFlexGroup justifyContent="flexStart">
+        <EuiFlexGroup gutterSize="s" justifyContent="spaceBetween" direction="column">
           <EuiFlexItem grow={false}>
-            <EuiButton
-              data-test-subj="alertsFlyoutPaginatePrevious"
-              fill
-              onClick={onPaginatePrevious}
-            >
-              {PREVIOUS_LABEL}
-            </EuiButton>
+            <EuiTitle size="xs">
+              <h4>{NAME_LABEL}</h4>
+            </EuiTitle>
+            <EuiSpacer size="s" />
+            {isLoading ? (
+              <EuiLoadingContent lines={1} />
+            ) : (
+              <EuiText size="s" data-test-subj="alertsFlyoutName">
+                {get(alert, AlertsField.name, [])[0]}
+              </EuiText>
+            )}
           </EuiFlexItem>
           <EuiFlexItem grow={false}>
-            <EuiButton data-test-subj="alertsFlyoutPaginateNext" fill onClick={onPaginateNext}>
-              {NEXT_LABEL}
-            </EuiButton>
+            <EuiTitle size="xs">
+              <h4>{REASON_LABEL}</h4>
+            </EuiTitle>
+            <EuiSpacer size="s" />
+            {isLoading ? (
+              <EuiLoadingContent lines={3} />
+            ) : (
+              <EuiText size="s" data-test-subj="alertsFlyoutReason">
+                {get(alert, AlertsField.reason, [])[0]}
+              </EuiText>
+            )}
           </EuiFlexItem>
         </EuiFlexGroup>
-      </EuiFlyoutFooter>
+        <EuiSpacer size="s" />
+        <EuiHorizontalRule size="full" />
+      </EuiFlyoutBody>
     </EuiFlyout>
   );
 };
diff --git a/...lugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table.test.tsx b/...lugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table.test.tsx
@@ -132,16 +132,16 @@ describe('AlertsTable', () => {
         const result = await wrapper.findAllByTestId('alertsFlyout');
         expect(result.length).toBe(1);
 
-        expect(wrapper.queryByTestId('alertsFlyoutTitle')?.textContent).toBe('one');
+        expect(wrapper.queryByTestId('alertsFlyoutName')?.textContent).toBe('one');
         expect(wrapper.queryByTestId('alertsFlyoutReason')?.textContent).toBe('two');
 
         // Should paginate too
-        userEvent.click(wrapper.queryAllByTestId('alertsFlyoutPaginateNext')[0]);
-        expect(wrapper.queryByTestId('alertsFlyoutTitle')?.textContent).toBe('three');
+        userEvent.click(wrapper.queryAllByTestId('pagination-button-next')[0]);
+        expect(wrapper.queryByTestId('alertsFlyoutName')?.textContent).toBe('three');
         expect(wrapper.queryByTestId('alertsFlyoutReason')?.textContent).toBe('four');
 
-        userEvent.click(wrapper.queryAllByTestId('alertsFlyoutPaginatePrevious')[0]);
-        expect(wrapper.queryByTestId('alertsFlyoutTitle')?.textContent).toBe('one');
+        userEvent.click(wrapper.queryAllByTestId('pagination-button-previous')[0]);
+        expect(wrapper.queryByTestId('alertsFlyoutName')?.textContent).toBe('one');
         expect(wrapper.queryByTestId('alertsFlyoutReason')?.textContent).toBe('two');
       });
 
@@ -152,10 +152,10 @@ describe('AlertsTable', () => {
         const result = await wrapper.findAllByTestId('alertsFlyout');
         expect(result.length).toBe(1);
 
-        userEvent.click(wrapper.queryAllByTestId('alertsFlyoutPaginateNext')[0]);
+        userEvent.click(wrapper.queryAllByTestId('pagination-button-next')[0]);
         expect(fetchAlertsData.onPageChange).toHaveBeenCalledWith({ pageIndex: 1, pageSize: 1 });
 
-        userEvent.click(wrapper.queryAllByTestId('alertsFlyoutPaginatePrevious')[0]);
+        userEvent.click(wrapper.queryAllByTestId('pagination-button-previous')[0]);
         expect(fetchAlertsData.onPageChange).toHaveBeenCalledWith({ pageIndex: 0, pageSize: 1 });
       });
     });

diff --git a/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table.tsx b/x-pack/plugins/triggers_actions_ui/public/application/sections/alerts_table/alerts_table.tsx
@@ -39,14 +39,14 @@ const emptyConfiguration = {
 
 const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTableProps) => {
   const [rowClasses, setRowClasses] = useState<EuiDataGridStyle['rowClasses']>({});
-  const { activePage, alertsCount, onPageChange, onSortChange } = props.useFetchAlertsData();
+  const { activePage, alertsCount, onPageChange, onSortChange, isLoading } =
+    props.useFetchAlertsData();
   const { sortingColumns, onSort } = useSorting(onSortChange);
   const {
     pagination,
     onChangePageSize,
     onChangePageIndex,
-    onPaginateFlyoutNext,
-    onPaginateFlyoutPrevious,
+    onPaginateFlyout,
     flyoutAlertIndex,
     setFlyoutAlertIndex,
   } = usePagination({
@@ -122,9 +122,11 @@ const AlertsTable: React.FunctionComponent<AlertsTableProps> = (props: AlertsTab
         <Suspense fallback={null}>
           <AlertsFlyout
             alert={props.alerts[flyoutAlertIndex]}
+            alertsCount={alertsCount}
             onClose={handleFlyoutClose}
-            onPaginateNext={onPaginateFlyoutNext}
-            onPaginatePrevious={onPaginateFlyoutPrevious}
+            flyoutIndex={flyoutAlertIndex + pagination.pageIndex * pagination.pageSize}
+            onPaginate={onPaginateFlyout}
+            isLoading={isLoading}
           />
         </Suspense>
       )}

diff --git a/...triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.test.ts b/...triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.test.ts
@@ -58,57 +58,53 @@ describe('usePagination', () => {
     expect(result.current.flyoutAlertIndex).toBe(-1);
 
     act(() => {
-      result.current.onPaginateFlyoutNext();
+      result.current.onPaginateFlyout(0);
     });
 
     expect(result.current.flyoutAlertIndex).toBe(0);
 
     act(() => {
-      result.current.onPaginateFlyoutNext();
+      result.current.onPaginateFlyout(1);
     });
 
     expect(result.current.flyoutAlertIndex).toBe(1);
 
     act(() => {
-      result.current.onPaginateFlyoutPrevious();
+      result.current.onPaginateFlyout(0);
     });
 
     expect(result.current.flyoutAlertIndex).toBe(0);
   });
 
-  it('should paginate the flyout when we need to change the page index', () => {
+  it('should paginate the flyout when we need to change the page index going back', () => {
     const { result } = renderHook(() =>
       usePagination({ onPageChange, pageIndex: 0, pageSize: 1, alertsCount })
     );
 
     act(() => {
-      result.current.onPaginateFlyoutPrevious();
+      result.current.onPaginateFlyout(-2);
     });
 
     // It should reset to the first alert in the table
     expect(result.current.flyoutAlertIndex).toBe(0);
 
     // It should go to the last page
     expect(result.current.pagination).toStrictEqual({ pageIndex: 4, pageSize: 1 });
+  });
 
-    act(() => {
-      result.current.onPaginateFlyoutNext();
-    });
-
-    // It should reset to the first alert in the table
-    expect(result.current.flyoutAlertIndex).toBe(0);
-
-    // It should go to the first page
-    expect(result.current.pagination).toStrictEqual({ pageIndex: 0, pageSize: 1 });
+  it('should paginate the flyout when we need to change the page index going forward', () => {
+    const { result } = renderHook(() =>
+      usePagination({ onPageChange, pageIndex: 0, pageSize: 1, alertsCount })
+    );
 
     act(() => {
-      result.current.onPaginateFlyoutNext();
+      result.current.onPaginateFlyout(1);
     });
 
     // It should reset to the first alert in the table
     expect(result.current.flyoutAlertIndex).toBe(0);
 
-    // It should go to the second page
+    // It should go to the first page
     expect(result.current.pagination).toStrictEqual({ pageIndex: 1, pageSize: 1 });
   });
 });
diff --git a/...gins/triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.ts b/...gins/triggers_actions_ui/public/application/sections/alerts_table/hooks/use_pagination.ts
@@ -58,19 +58,20 @@ export function usePagination({ onPageChange, pageIndex, pageSize, alertsCount }
     },
     [pagination, alertsCount, onChangePageIndex]
   );
-  const onPaginateFlyoutNext = useCallback(() => {
-    paginateFlyout(flyoutAlertIndex + 1);
-  }, [paginateFlyout, flyoutAlertIndex]);
-  const onPaginateFlyoutPrevious = useCallback(() => {
-    paginateFlyout(flyoutAlertIndex - 1);
-  }, [paginateFlyout, flyoutAlertIndex]);
+
+  const onPaginateFlyout = useCallback(
+    (nextPageIndex: number) => {
+      nextPageIndex -= pagination.pageSize * pagination.pageIndex;
+      paginateFlyout(nextPageIndex);
+    },
+    [paginateFlyout, pagination.pageSize, pagination.pageIndex]
+  );
 
   return {
     pagination,
     onChangePageSize,
     onChangePageIndex,
-    onPaginateFlyoutNext,
-    onPaginateFlyoutPrevious,
+    onPaginateFlyout,
     flyoutAlertIndex,
     setFlyoutAlertIndex,
   };