[App Search] Results follow-up (elastic#96709)

* CSS cleanup

* Refactor ResultActions component + DRY out link behavior

- Create new separate ResultActions component
- Pass actions array through to header and have haeder in charge of conditional visibility / FlexItem wrapper (this matches the other header items)
- shouldLinkToDetailPage: instead of generating custom JSX, just have it be a standard action and append it to the actions array

Link behavior:
- ResultHeaderItem - switch to EuiLinkTo, no need for extra wrapper
- ResultHeader - DRY out unnecessary extra path generation - instead pass down a conditional documentLink instead of a bool

* PR feedback: Fix test name

* PR feedback: unshift

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.scss

@@ -1,10 +1,10 @@
 .appSearchResult {
   display: grid;
-  grid-template-columns: auto 1fr auto;
-  grid-template-rows: auto 1fr auto;
+  grid-template-columns: auto 1fr;
+  grid-template-rows: auto 1fr;
   grid-template-areas:
-    'drag content actions'
-    'drag toggle actions';
+    'drag content'
+    'drag toggle';
   overflow: hidden; // Prevents child background-colors from clipping outside of panel border-radius
   border: $euiBorderThin; // TODO: Remove after EUI version is bumped beyond 31.8.0
 
@@ -35,29 +35,6 @@
     }
   }
 
-  &__actionButtons {
-    grid-area: actions;
-    display: flex;
-    flex-wrap: no-wrap;
-  }
-
-  &__actionButton {
-    display: flex;
-    justify-content: center;
-    align-items: center;
-    width: $euiSize * 2;
-    border-left: $euiBorderThin;
-
-    &:first-child {
-      border-left: none;
-    }
-
-    &:hover,
-    &:focus {
-      background-color: $euiPageBackgroundColor;
-    }
-  }
-
   &__dragHandle {
     grid-area: drag;
     display: flex;

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.test.tsx

@@ -5,12 +5,14 @@
  * 2.0.
  */
 
+import { mockKibanaValues } from '../../../__mocks__';
+
 import React from 'react';
 import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd';
 
 import { shallow, ShallowWrapper } from 'enzyme';
 
-import { EuiButtonIcon, EuiPanel, EuiButtonIconColor } from '@elastic/eui';
+import { EuiPanel } from '@elastic/eui';
 
 import { SchemaTypes } from '../../../shared/types';
 
@@ -63,18 +65,28 @@ describe('Result', () => {
     ]);
   });
 
-  it('renders a header', () => {
-    const wrapper = shallow(<Result {...props} showScore isMetaEngine />);
-    const header = wrapper.find(ResultHeader);
-    expect(header.exists()).toBe(true);
-    expect(header.prop('isMetaEngine')).toBe(true); // passed through from props
-    expect(header.prop('showScore')).toBe(true); // passed through from props
-    expect(header.prop('shouldLinkToDetailPage')).toBe(false); // passed through from props
-    expect(header.prop('resultMeta')).toEqual({
-      id: '1',
-      score: 100,
-      engine: 'my-engine',
-    }); // passed through from meta in result prop
+  describe('header', () => {
+    it('renders a header', () => {
+      const wrapper = shallow(<Result {...props} showScore isMetaEngine />);
+      const header = wrapper.find(ResultHeader);
+
+      expect(header.exists()).toBe(true);
+      expect(header.prop('isMetaEngine')).toBe(true); // passed through from props
+      expect(header.prop('showScore')).toBe(true); // passed through from props
+      expect(header.prop('resultMeta')).toEqual({
+        id: '1',
+        score: 100,
+        engine: 'my-engine',
+      }); // passed through from meta in result prop
+      expect(header.prop('documentLink')).toBe(undefined); // based on shouldLinkToDetailPage prop
+    });
+
+    it('passes documentLink when shouldLinkToDetailPage is true', () => {
+      const wrapper = shallow(<Result {...props} shouldLinkToDetailPage />);
+      const header = wrapper.find(ResultHeader);
+
+      expect(header.prop('documentLink')).toBe('/engines/my-engine/documents/1');
+    });
   });
 
   describe('actions', () => {
@@ -83,53 +95,30 @@ describe('Result', () => {
         title: 'Hide',
         onClick: jest.fn(),
         iconType: 'eyeClosed',
-        iconColor: 'danger' as EuiButtonIconColor,
       },
       {
         title: 'Bookmark',
         onClick: jest.fn(),
         iconType: 'starFilled',
-        iconColor: undefined,
       },
     ];
 
-    it('will render an action button in the header for each action passed', () => {
+    it('passes actions to the header', () => {
       const wrapper = shallow(<Result {...props} actions={actions} />);
-      const header = wrapper.find(ResultHeader);
-      const renderedActions = shallow(header.prop('actions') as any);
-      const buttons = renderedActions.find(EuiButtonIcon);
-      expect(buttons).toHaveLength(2);
-
-      expect(buttons.first().prop('iconType')).toEqual('eyeClosed');
-      expect(buttons.first().prop('color')).toEqual('danger');
-      buttons.first().simulate('click');
-      expect(actions[0].onClick).toHaveBeenCalled();
-
-      expect(buttons.last().prop('iconType')).toEqual('starFilled');
-      // Note that no iconColor was passed so it was defaulted to primary
-      expect(buttons.last().prop('color')).toEqual('primary');
-      buttons.last().simulate('click');
-      expect(actions[1].onClick).toHaveBeenCalled();
+      expect(wrapper.find(ResultHeader).prop('actions')).toEqual(actions);
     });
 
-    it('will render a document detail link as the first action if shouldLinkToDetailPage is passed', () => {
+    it('adds a link action to the start of the actions array if shouldLinkToDetailPage is passed', () => {
       const wrapper = shallow(<Result {...props} actions={actions} shouldLinkToDetailPage />);
-      const header = wrapper.find(ResultHeader);
-      const renderedActions = shallow(header.prop('actions') as any);
-      const buttons = renderedActions.find(EuiButtonIcon);
 
-      // In addition to the 2 actions passed, we also have a link action
-      expect(buttons).toHaveLength(3);
+      const passedActions = wrapper.find(ResultHeader).prop('actions');
+      expect(passedActions.length).toEqual(3); // In addition to the 2 actions passed, we also have a link action
 
-      expect(buttons.first().prop('data-test-subj')).toEqual('DocumentDetailLink');
-    });
+      const linkAction = passedActions[0];
+      expect(linkAction.title).toEqual('Visit document details');
 
-    it('will not render anything if no actions are passed and shouldLinkToDetailPage is false', () => {
-      const wrapper = shallow(<Result {...props} actions={undefined} />);
-      const header = wrapper.find(ResultHeader);
-      const renderedActions = shallow(header.prop('actions') as any);
-      const buttons = renderedActions.find(EuiButtonIcon);
-      expect(buttons).toHaveLength(0);
+      linkAction.onClick();
+      expect(mockKibanaValues.navigateToUrl).toHaveBeenCalledWith('/engines/my-engine/documents/1');
     });
   });
 
@@ -148,9 +137,7 @@ describe('Result', () => {
   });
 
   it('will render field details with type highlights if schemaForTypeHighlights has been provided', () => {
-    const wrapper = shallow(
-      <Result {...props} shouldLinkToDetailPage schemaForTypeHighlights={schema} />
-    );
+    const wrapper = shallow(<Result {...props} schemaForTypeHighlights={schema} />);
     expect(wrapper.find(ResultField).map((rf) => rf.prop('type'))).toEqual([
       'text',
       'text',

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result.tsx

@@ -10,12 +10,11 @@ import { DraggableProvidedDragHandleProps } from 'react-beautiful-dnd';
 
 import './result.scss';
 
-import { EuiButtonIcon, EuiPanel, EuiFlexGroup, EuiFlexItem, EuiIcon } from '@elastic/eui';
+import { EuiPanel, EuiIcon } from '@elastic/eui';
 
 import { i18n } from '@kbn/i18n';
 
-import { ReactRouterHelper } from '../../../shared/react_router_helpers/eui_components';
-
+import { KibanaLogic } from '../../../shared/kibana';
 import { Schema } from '../../../shared/types';
 
 import { ENGINE_DOCUMENT_DETAIL_PATH } from '../../routes';
@@ -56,48 +55,27 @@ export const Result: React.FC<Props> = ({
     [result]
   );
   const numResults = resultFields.length;
-  const documentLink = generateEncodedPath(ENGINE_DOCUMENT_DETAIL_PATH, {
-    engineName: resultMeta.engine,
-    documentId: resultMeta.id,
-  });
 
   const typeForField = (fieldName: string) => {
     if (schemaForTypeHighlights) return schemaForTypeHighlights[fieldName];
   };
 
-  const ResultActions = () => {
-    if (!shouldLinkToDetailPage && !actions.length) return null;
-    return (
-      <EuiFlexItem grow={false}>
-        <EuiFlexGroup gutterSize="s">
-          {shouldLinkToDetailPage && (
-            <ReactRouterHelper to={documentLink}>
-              <EuiFlexItem grow={false}>
-                <EuiButtonIcon
-                  iconType="eye"
-                  data-test-subj="DocumentDetailLink"
-                  aria-label={i18n.translate(
-                    'xpack.enterpriseSearch.appSearch.result.documentDetailLink',
-                    { defaultMessage: 'Visit document details' }
-                  )}
-                />
-              </EuiFlexItem>
-            </ReactRouterHelper>
-          )}
-          {actions.map(({ onClick, title, iconType, iconColor }) => (
-            <EuiFlexItem key={title}>
-              <EuiButtonIcon
-                iconType={iconType}
-                onClick={onClick}
-                color={iconColor ? iconColor : 'primary'}
-                aria-label={title}
-              />
-            </EuiFlexItem>
-          ))}
-        </EuiFlexGroup>
-      </EuiFlexItem>
-    );
-  };
+  const documentLink = shouldLinkToDetailPage
+    ? generateEncodedPath(ENGINE_DOCUMENT_DETAIL_PATH, {
+        engineName: resultMeta.engine,
+        documentId: resultMeta.id,
+      })
+    : undefined;
+  if (shouldLinkToDetailPage && documentLink) {
+    const linkAction = {
+      onClick: () => KibanaLogic.values.navigateToUrl(documentLink),
+      title: i18n.translate('xpack.enterpriseSearch.appSearch.result.documentDetailLink', {
+        defaultMessage: 'Visit document details',
+      }),
+      iconType: 'eye',
+    };
+    actions = [linkAction, ...actions];
+  }
 
   return (
     <EuiPanel
@@ -120,8 +98,8 @@ export const Result: React.FC<Props> = ({
           resultMeta={resultMeta}
           showScore={!!showScore}
           isMetaEngine={isMetaEngine}
-          shouldLinkToDetailPage={shouldLinkToDetailPage}
-          actions={<ResultActions />}
+          documentLink={documentLink}
+          actions={actions}
         />
         {resultFields
           .slice(0, isOpen ? resultFields.length : RESULT_CUTOFF)

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result_actions.test.tsx

@@ -0,0 +1,55 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import React from 'react';
+
+import { shallow } from 'enzyme';
+
+import { EuiButtonIcon, EuiButtonIconColor } from '@elastic/eui';
+
+import { ResultActions } from './result_actions';
+
+describe('ResultActions', () => {
+  const actions = [
+    {
+      title: 'Hide',
+      onClick: jest.fn(),
+      iconType: 'eyeClosed',
+      iconColor: 'danger' as EuiButtonIconColor,
+    },
+    {
+      title: 'Bookmark',
+      onClick: jest.fn(),
+      iconType: 'starFilled',
+      iconColor: undefined,
+    },
+  ];
+
+  const wrapper = shallow(<ResultActions actions={actions} />);
+  const buttons = wrapper.find(EuiButtonIcon);
+
+  it('renders an action button for each action passed', () => {
+    expect(buttons).toHaveLength(2);
+  });
+
+  it('passes icon props correctly', () => {
+    expect(buttons.first().prop('iconType')).toEqual('eyeClosed');
+    expect(buttons.first().prop('color')).toEqual('danger');
+
+    expect(buttons.last().prop('iconType')).toEqual('starFilled');
+    // Note that no iconColor was passed so it was defaulted to primary
+    expect(buttons.last().prop('color')).toEqual('primary');
+  });
+
+  it('passes click events', () => {
+    buttons.first().simulate('click');
+    expect(actions[0].onClick).toHaveBeenCalled();
+
+    buttons.last().simulate('click');
+    expect(actions[1].onClick).toHaveBeenCalled();
+  });
+});

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result_actions.tsx

@@ -0,0 +1,34 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import React from 'react';
+
+import { EuiButtonIcon, EuiFlexGroup, EuiFlexItem } from '@elastic/eui';
+
+import { ResultAction } from './types';
+
+interface Props {
+  actions: ResultAction[];
+}
+
+export const ResultActions: React.FC<Props> = ({ actions }) => {
+  return (
+    <EuiFlexGroup gutterSize="s" responsive={false}>
+      {actions.map(({ onClick, title, iconType, iconColor }) => (
+        <EuiFlexItem key={title} grow={false}>
+          <EuiButtonIcon
+            iconType={iconType}
+            onClick={onClick}
+            color={iconColor ? iconColor : 'primary'}
+            aria-label={title}
+            title={title}
+          />
+        </EuiFlexItem>
+      ))}
+    </EuiFlexGroup>
+  );
+};

x-pack/plugins/enterprise_search/public/applications/app_search/components/result/result_header.scss

@@ -1,26 +1,3 @@
 .appSearchResultHeader {
-  display: flex;
-  margin-bottom: $euiSizeS;
-
-  @include euiBreakpoint('xs') {
-    flex-direction: column;
-  }
-
-  &__column {
-    display: flex;
-    flex-wrap: wrap;
-
-    @include euiBreakpoint('xs') {
-      flex-direction: column;
-    }
-
-    & + &,
-    .appSearchResultHeaderItem + .appSearchResultHeaderItem {
-      margin-left: $euiSizeL;
-
-      @include euiBreakpoint('xs') {
-        margin-left: 0;
-      }
-    }
-  }
+  margin-bottom: $euiSizeM;
 }