refactor ViewDetailsPopover

elastic · Oct 26, 2023 · f0a6f43 · f0a6f43
1 parent d03388a
commit f0a6f43
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 58 deletions.
diff --git a/...ges/kbn-search-response-warnings/src/components/search_response_warnings/empty_prompt.tsx b/...ges/kbn-search-response-warnings/src/components/search_response_warnings/empty_prompt.tsx
@@ -30,7 +30,7 @@ export const SearchResponseWarningsEmptyPrompt = (props: Props) => {
         </h2>
       }
       body={getWarningsDescription(props.warnings)}
-      actions={<ViewDetailsPopover displayAsLink={false} warnings={props.warnings} />}
+      actions={<ViewDetailsPopover warnings={props.warnings} />}
       data-test-subj="searchResponseWarningsEmptyPrompt"
     />
   );

diff --git a/...h-response-warnings/src/components/search_response_warnings/view_details_popover.test.tsx b/...h-response-warnings/src/components/search_response_warnings/view_details_popover.test.tsx
@@ -13,60 +13,90 @@ import type { SearchResponseWarning } from '../../types';
 
 describe('ViewDetailsPopover', () => {
   describe('single warning', () => {
-    test('Clicking "view details" should open warning details', () => {
-      const mockOpenInInspector = jest.fn();
-      const warnings = [
-        {
-          type: 'incomplete',
-          requestName: 'My request',
-          clusters: {
-            remote1: {
-              status: 'partial',
-              indices: '',
-              timed_out: false,
-            },
+    const mockOpenInInspector = jest.fn();
+    const warnings = [
+      {
+        type: 'incomplete',
+        requestName: 'My request',
+        clusters: {
+          remote1: {
+            status: 'partial',
+            indices: '',
+            timed_out: false,
           },
-          openInInspector: mockOpenInInspector,
-        } as SearchResponseWarning,
-      ];
-      render(<ViewDetailsPopover displayAsLink={false} warnings={warnings} />);
+        },
+        openInInspector: mockOpenInInspector,
+      } as SearchResponseWarning,
+    ];
+
+    beforeEach(() => {
+      mockOpenInInspector.mockReset();
+    });
+
+    test('Clicking "view details" button should open warning details', () => {
+      render(<ViewDetailsPopover warnings={warnings} />);
+      const viewDetailsButton = screen.getByRole('button');
+      fireEvent.click(viewDetailsButton);
+      expect(mockOpenInInspector).toHaveBeenCalled();
+    });
+
+    test('Clicking "view details" link should open warning details', () => {
+      render(<ViewDetailsPopover displayAsLink={true} warnings={warnings} />);
       const viewDetailsButton = screen.getByRole('button');
       fireEvent.click(viewDetailsButton);
       expect(mockOpenInInspector).toHaveBeenCalled();
     });
   });
 
   describe('multiple warnings', () => {
-    test('Clicking "view details" should open popover with button to view details for each warning', () => {
-      const request1MockOpenInInspector = jest.fn();
-      const request2MockOpenInInspector = jest.fn();
-      const warnings = [
-        {
-          type: 'incomplete',
-          requestName: 'My first request',
-          clusters: {
-            remote1: {
-              status: 'partial',
-              indices: '',
-              timed_out: false,
-            },
+    const request1MockOpenInInspector = jest.fn();
+    const request2MockOpenInInspector = jest.fn();
+    const warnings = [
+      {
+        type: 'incomplete',
+        requestName: 'My first request',
+        clusters: {
+          remote1: {
+            status: 'partial',
+            indices: '',
+            timed_out: false,
           },
-          openInInspector: request1MockOpenInInspector,
-        } as SearchResponseWarning,
-        {
-          type: 'incomplete',
-          requestName: 'My second request',
-          clusters: {
-            remote1: {
-              status: 'partial',
-              indices: '',
-              timed_out: false,
-            },
+        },
+        openInInspector: request1MockOpenInInspector,
+      } as SearchResponseWarning,
+      {
+        type: 'incomplete',
+        requestName: 'My second request',
+        clusters: {
+          remote1: {
+            status: 'partial',
+            indices: '',
+            timed_out: false,
           },
-          openInInspector: request2MockOpenInInspector,
-        } as SearchResponseWarning,
-      ];
-      render(<ViewDetailsPopover displayAsLink={false} warnings={warnings} />);
+        },
+        openInInspector: request2MockOpenInInspector,
+      } as SearchResponseWarning,
+    ];
+    beforeEach(() => {
+      request1MockOpenInInspector.mockReset();
+      request2MockOpenInInspector.mockReset();
+    });
+
+    test('Clicking "view details" button should open popover with button to view details for each warning', () => {
+      render(<ViewDetailsPopover warnings={warnings} />);
+      const viewDetailsButton = screen.getByRole('button');
+      fireEvent.click(viewDetailsButton);
+      expect(request1MockOpenInInspector).not.toHaveBeenCalled();
+      expect(request2MockOpenInInspector).not.toHaveBeenCalled();
+
+      const openRequest1Button = screen.getByRole('button', { name: 'My first request' });
+      fireEvent.click(openRequest1Button);
+      expect(request1MockOpenInInspector).toHaveBeenCalled();
+      expect(request2MockOpenInInspector).not.toHaveBeenCalled();
+    });
+
+    test('Clicking "view details" link should open popover with button to view details for each warning', () => {
+      render(<ViewDetailsPopover displayAsLink={true} warnings={warnings} />);
       const viewDetailsButton = screen.getByRole('button');
       fireEvent.click(viewDetailsButton);
       expect(request1MockOpenInInspector).not.toHaveBeenCalled();

diff --git a/...search-response-warnings/src/components/search_response_warnings/view_details_popover.tsx b/...search-response-warnings/src/components/search_response_warnings/view_details_popover.tsx
@@ -19,25 +19,29 @@ import { viewDetailsLabel } from './i18n_utils';
 import type { SearchResponseWarning } from '../../types';
 
 interface Props {
-  displayAsLink: boolean;
+  displayAsLink?: boolean;
   warnings: SearchResponseWarning[];
 }
 
 export const ViewDetailsPopover = (props: Props) => {
   const [isPopoverOpen, setIsPopoverOpen] = useState(false);
 
-  const ActionComponent = props.displayAsLink ? EuiLink : EuiButton;
-
   if (!props.warnings.length) {
     return null;
   }
 
   if (props.warnings.length === 1) {
-    return (
-      <ActionComponent color="primary" onClick={props.warnings[0].openInInspector}>
-        {viewDetailsLabel}
-      </ActionComponent>
-    );
+    return props.displayAsLink
+      ? (
+          <EuiLink color="primary" onClick={props.warnings[0].openInInspector}>
+            {viewDetailsLabel}
+          </EuiLink>
+        )
+      : (
+          <EuiButton color="primary" onClick={props.warnings[0].openInInspector}>
+            {viewDetailsLabel}
+          </EuiButton>
+        );
   }
 
   const panels: EuiContextMenuPanelDescriptor[] = [
@@ -59,11 +63,19 @@ export const ViewDetailsPopover = (props: Props) => {
     <EuiPopover
       id="ViewDetailsPopover"
       button={
-        <ActionComponent color="primary" onClick={() => setIsPopoverOpen(!isPopoverOpen)}>
-          <>
-            {viewDetailsLabel} <EuiIcon type="arrowRight" size="s" />
-          </>
-        </ActionComponent>
+        props.displayAsLink
+          ? (
+              <EuiLink color="primary" onClick={() => setIsPopoverOpen(!isPopoverOpen)}>
+                <>
+                  {viewDetailsLabel} <EuiIcon type="arrowRight" size="s" />
+                </>
+              </EuiLink>
+            )
+          : (
+              <EuiButton color="primary" onClick={() => setIsPopoverOpen(!isPopoverOpen)} iconSide="right" iconType="arrowRight">
+                {viewDetailsLabel}
+              </EuiButton>
+            )
       }
       isOpen={isPopoverOpen}
       closePopover={() => setIsPopoverOpen(false)}