APM Service Map popover fixes round 1 (#56502) (#56568)

Some fixes for #54405. * Use the `popoverPositionFluid` method of `EuiPopover` to properly position the arrow based on where the popover was opened * Rearrange the variable declarations and effects in the popover index * Create a `Contents` component to separate the internals of the popover from the index, where the poisitioning is done * Add a story to show the contents rendering Co-authored-by: Elastic Machine <[email protected]>
elastic · Feb 5, 2020 · 1abc562 · 1abc562
1 parent 0855f12
commit 1abc562
Show file tree

Hide file tree

Showing 4 changed files with 171 additions and 77 deletions.
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Contents.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Contents.tsx
@@ -0,0 +1,66 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import {
+  EuiFlexGroup,
+  EuiFlexItem,
+  EuiHorizontalRule,
+  EuiTitle
+} from '@elastic/eui';
+import cytoscape from 'cytoscape';
+import React from 'react';
+import { Buttons } from './Buttons';
+import { Info } from './Info';
+import { ServiceMetricList } from './ServiceMetricList';
+
+const popoverMinWidth = 280;
+
+interface ContentsProps {
+  focusedServiceName?: string;
+  isService: boolean;
+  label: string;
+  onFocusClick: () => void;
+  selectedNodeData: cytoscape.NodeDataDefinition;
+  selectedNodeServiceName: string;
+}
+
+export function Contents({
+  selectedNodeData,
+  focusedServiceName,
+  isService,
+  label,
+  onFocusClick,
+  selectedNodeServiceName
+}: ContentsProps) {
+  return (
+    <EuiFlexGroup
+      direction="column"
+      gutterSize="s"
+      style={{ minWidth: popoverMinWidth }}
+    >
+      <EuiFlexItem>
+        <EuiTitle size="xxs">
+          <h3>{label}</h3>
+        </EuiTitle>
+        <EuiHorizontalRule margin="xs" />
+      </EuiFlexItem>
+      <EuiFlexItem>
+        {isService ? (
+          <ServiceMetricList serviceName={selectedNodeServiceName} />
+        ) : (
+          <Info {...selectedNodeData} />
+        )}
+      </EuiFlexItem>
+      {isService && (
+        <Buttons
+          focusedServiceName={focusedServiceName}
+          onFocusClick={onFocusClick}
+          selectedNodeServiceName={selectedNodeServiceName}
+        />
+      )}
+    </EuiFlexGroup>
+  );
+}
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Info.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Info.tsx
@@ -4,10 +4,11 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import React from 'react';
+import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
 import { i18n } from '@kbn/i18n';
+import cytoscape from 'cytoscape';
+import React from 'react';
 import styled from 'styled-components';
-import lightTheme from '@elastic/eui/dist/eui_theme_light.json';
 
 const ItemRow = styled.div`
   line-height: 2;
@@ -19,8 +20,8 @@ const ItemTitle = styled.dt`
 
 const ItemDescription = styled.dd``;
 
-interface InfoProps {
-  type: string;
+interface InfoProps extends cytoscape.NodeDataDefinition {
+  type?: string;
   subtype?: string;
 }
 

diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Popover.stories.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/Popover.stories.tsx
@@ -0,0 +1,49 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { storiesOf } from '@storybook/react';
+import React from 'react';
+import {
+  ApmPluginContext,
+  ApmPluginContextValue
+} from '../../../../context/ApmPluginContext';
+import { Contents } from './Contents';
+
+const selectedNodeData = {
+  id: 'opbeans-node',
+  label: 'opbeans-node',
+  href:
+    '#/services/opbeans-node/service-map?rangeFrom=now-24h&rangeTo=now&refreshPaused=true&refreshInterval=0',
+  agentName: 'nodejs',
+  type: 'service'
+};
+
+storiesOf('app/ServiceMap/Popover/Contents', module).add(
+  'example',
+  () => {
+    return (
+      <ApmPluginContext.Provider
+        value={
+          ({ core: { notifications: {} } } as unknown) as ApmPluginContextValue
+        }
+      >
+        <Contents
+          selectedNodeData={selectedNodeData}
+          isService={true}
+          label="opbeans-node"
+          onFocusClick={() => {}}
+          selectedNodeServiceName="opbeans-node"
+        />
+      </ApmPluginContext.Provider>
+    );
+  },
+  {
+    info: {
+      propTablesExclude: [ApmPluginContext.Provider],
+      source: false
+    }
+  }
+);
diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/index.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceMap/Popover/index.tsx
@@ -4,27 +4,18 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import {
-  EuiFlexGroup,
-  EuiFlexItem,
-  EuiHorizontalRule,
-  EuiPopover,
-  EuiTitle
-} from '@elastic/eui';
+import { EuiPopover } from '@elastic/eui';
 import cytoscape from 'cytoscape';
 import React, {
   CSSProperties,
+  useCallback,
   useContext,
   useEffect,
-  useState,
-  useCallback
+  useRef,
+  useState
 } from 'react';
 import { CytoscapeContext } from '../Cytoscape';
-import { Buttons } from './Buttons';
-import { Info } from './Info';
-import { ServiceMetricList } from './ServiceMetricList';
-
-const popoverMinWidth = 280;
+import { Contents } from './Contents';
 
 interface PopoverProps {
   focusedServiceName?: string;
@@ -35,92 +26,79 @@ export function Popover({ focusedServiceName }: PopoverProps) {
   const [selectedNode, setSelectedNode] = useState<
     cytoscape.NodeSingular | undefined
   >(undefined);
-  const onFocusClick = useCallback(() => setSelectedNode(undefined), [
+  const deselect = useCallback(() => setSelectedNode(undefined), [
     setSelectedNode
   ]);
-
-  useEffect(() => {
-    const selectHandler: cytoscape.EventHandler = event => {
-      setSelectedNode(event.target);
-    };
-    const unselectHandler: cytoscape.EventHandler = () => {
-      setSelectedNode(undefined);
-    };
-
-    if (cy) {
-      cy.on('select', 'node', selectHandler);
-      cy.on('unselect', 'node', unselectHandler);
-      cy.on('data viewport', unselectHandler);
-    }
-
-    return () => {
-      if (cy) {
-        cy.removeListener('select', 'node', selectHandler);
-        cy.removeListener('unselect', 'node', unselectHandler);
-        cy.removeListener('data viewport', undefined, unselectHandler);
-      }
-    };
-  }, [cy]);
-
   const renderedHeight = selectedNode?.renderedHeight() ?? 0;
   const renderedWidth = selectedNode?.renderedWidth() ?? 0;
   const { x, y } = selectedNode?.renderedPosition() ?? { x: 0, y: 0 };
   const isOpen = !!selectedNode;
-  const selectedNodeServiceName: string = selectedNode?.data('id');
   const isService = selectedNode?.data('type') === 'service';
   const triggerStyle: CSSProperties = {
     background: 'transparent',
     height: renderedHeight,
     position: 'absolute',
-    width: renderedWidth
+    width: renderedWidth,
+    border: '3px dotted red'
   };
-  const trigger = <div className="trigger" style={triggerStyle} />;
-
+  const trigger = <div style={triggerStyle} />;
   const zoom = cy?.zoom() ?? 1;
   const height = selectedNode?.height() ?? 0;
-  const translateY = y - (zoom + 1) * (height / 2);
+  const translateY = y - ((zoom + 1) * height) / 4;
   const popoverStyle: CSSProperties = {
     position: 'absolute',
     transform: `translate(${x}px, ${translateY}px)`
   };
-  const data = selectedNode?.data() ?? {};
-  const label = data.label || selectedNodeServiceName;
+  const selectedNodeData = selectedNode?.data() ?? {};
+  const selectedNodeServiceName = selectedNodeData.id;
+  const label = selectedNodeData.label || selectedNodeServiceName;
+  const popoverRef = useRef<EuiPopover>(null);
+
+  // Set up Cytoscape event handlers
+  useEffect(() => {
+    const selectHandler: cytoscape.EventHandler = event => {
+      setSelectedNode(event.target);
+    };
+
+    if (cy) {
+      cy.on('select', 'node', selectHandler);
+      cy.on('unselect', 'node', deselect);
+      cy.on('data viewport', deselect);
+    }
+
+    return () => {
+      if (cy) {
+        cy.removeListener('select', 'node', selectHandler);
+        cy.removeListener('unselect', 'node', deselect);
+        cy.removeListener('data viewport', undefined, deselect);
+      }
+    };
+  }, [cy, deselect]);
+
+  // Handle positioning of popover. This makes it so the popover positions
+  // itself correctly and the arrows are always pointing to where they should.
+  useEffect(() => {
+    if (popoverRef.current) {
+      popoverRef.current.positionPopoverFluid();
+    }
+  }, [popoverRef, x, y]);
 
   return (
     <EuiPopover
       anchorPosition={'upCenter'}
       button={trigger}
       closePopover={() => {}}
       isOpen={isOpen}
+      ref={popoverRef}
       style={popoverStyle}
     >
-      <EuiFlexGroup
-        direction="column"
-        gutterSize="s"
-        style={{ minWidth: popoverMinWidth }}
-      >
-        <EuiFlexItem>
-          <EuiTitle size="xxs">
-            <h3>{label}</h3>
-          </EuiTitle>
-          <EuiHorizontalRule margin="xs" />
-        </EuiFlexItem>
-
-        <EuiFlexItem>
-          {isService ? (
-            <ServiceMetricList serviceName={selectedNodeServiceName} />
-          ) : (
-            <Info {...data} />
-          )}
-        </EuiFlexItem>
-        {isService && (
-          <Buttons
-            focusedServiceName={focusedServiceName}
-            onFocusClick={onFocusClick}
-            selectedNodeServiceName={selectedNodeServiceName}
-          />
-        )}
-      </EuiFlexGroup>
+      <Contents
+        selectedNodeData={selectedNodeData}
+        isService={isService}
+        label={label}
+        onFocusClick={deselect}
+        selectedNodeServiceName={selectedNodeServiceName}
+      />
     </EuiPopover>
   );
 }