address comments

Signed-off-by: Adam Tackett <[email protected]>
opensearch-project · Dec 4, 2024 · 456e9fd · 456e9fd
1 parent 7f361b1
commit 456e9fd
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 57 deletions.
diff --git a/public/components/trace_analytics/components/common/helper_functions.tsx b/public/components/trace_analytics/components/common/helper_functions.tsx
@@ -596,6 +596,7 @@ interface FullScreenWrapperProps {
   isFullScreen: boolean;
 }
 
+// EUI Data grid full screen button is currently broken, this is a workaround
 export const FullScreenWrapper: React.FC<FullScreenWrapperProps> = ({
   children,
   onClose,
@@ -605,39 +606,15 @@ export const FullScreenWrapper: React.FC<FullScreenWrapperProps> = ({
 
   return (
     <EuiOverlayMask>
-      <div
-        style={{
-          position: 'fixed',
-          top: 0,
-          left: 0,
-          width: '100vw',
-          height: '100vh',
-          backgroundColor: '#fff',
-          zIndex: 9999,
-          display: 'flex',
-          flexDirection: 'column',
-        }}
-      >
+      <div className="full-screen-wrapper">
         <EuiButtonIcon
           iconType="cross"
           aria-label="Close full screen"
           onClick={onClose}
           display="empty"
-          style={{
-            position: 'absolute',
-            top: '4px',
-            right: '4px',
-            zIndex: 10000,
-          }}
+          className="full-screen-close-icon"
         />
-        <div
-          style={{
-            flex: '1 1 auto',
-            overflow: 'auto',
-          }}
-        >
-          {children}
-        </div>
+        <div className="full-screen-content">{children}</div>
       </div>
     </EuiOverlayMask>
   );

diff --git a/public/components/trace_analytics/components/services/services_content.tsx b/public/components/trace_analytics/components/services/services_content.tsx
@@ -61,10 +61,6 @@ export function ServicesContent(props: ServicesProps) {
   const [serviceTrends, setServiceTrends] = useState<ServiceTrends>({});
   const searchBarRef = useRef<{ updateQuery: (newQuery: string) => void }>(null);
 
-  useEffect(() => {
-    refresh();
-  }, [startTime, endTime, props.dataSourceMDSId]);
-
   useEffect(() => {
     const isNavGroupEnabled = coreRefs?.chrome?.navGroup.getNavGroupEnabled();
     chrome.setBreadcrumbs([...(isNavGroupEnabled ? [] : [parentBreadcrumb]), ...childBreadcrumbs]);
@@ -104,6 +100,9 @@ export function ServicesContent(props: ServicesProps) {
     jaegerIndicesExist,
     dataPrepperIndicesExist,
     isServiceTrendEnabled,
+    startTime,
+    endTime,
+    props.dataSourceMDSId,
   ]);
 
   const refresh = async (currService?: string, overrideQuery?: string) => {

diff --git a/...trace_analytics/components/traces/__tests__/__snapshots__/span_detail_table.test.tsx.snap b/...trace_analytics/components/traces/__tests__/__snapshots__/span_detail_table.test.tsx.snap
@@ -614,6 +614,7 @@ exports[`SpanDetailTableHierarchy renders the empty component 1`] = `
             </EuiButtonEmpty>,
             <EuiButtonEmpty
               color="text"
+              data-test-subj="treeExpandAll"
               iconType="expand"
               onClick={[Function]}
               size="xs"
@@ -622,6 +623,7 @@ exports[`SpanDetailTableHierarchy renders the empty component 1`] = `
             </EuiButtonEmpty>,
             <EuiButtonEmpty
               color="text"
+              data-test-subj="treeCollapseAll"
               iconType="minimize"
               onClick={[Function]}
               size="xs"

diff --git a/public/components/trace_analytics/components/traces/__tests__/span_detail_panel.test.tsx b/public/components/trace_analytics/components/traces/__tests__/span_detail_panel.test.tsx
@@ -7,7 +7,7 @@ import React from 'react';
 import { configure, mount, shallow } from 'enzyme';
 import Adapter from 'enzyme-adapter-react-16';
 import { SpanDetailPanel } from '../span_detail_panel';
-import { EuiButtonGroup, EuiSmallButton } from '@elastic/eui';
+import { EuiSmallButton } from '@elastic/eui';
 import { Plt } from '../../../../visualizations/plotly/plot';
 import { act } from 'react-dom/test-utils';
 
@@ -128,26 +128,6 @@ describe('SpanDetailPanel component', () => {
     expect(resetButton.prop('isDisabled')).toBe(true);
   });
 
-  it('handles view toggle button group', () => {
-    const wrapper = mount(<SpanDetailPanel {...mockProps} />);
-    const toggleButtons = wrapper.find(EuiButtonGroup);
-    expect(toggleButtons).toHaveLength(1);
-
-    // Verify initial state is 'timeline'
-    expect(toggleButtons.prop('idSelected')).toBe('timeline');
-
-    // Simulate changing the toggle
-    act(() => {
-      toggleButtons.prop('onChange')!('span_list');
-    });
-
-    wrapper.update();
-
-    // Verify the toggle button group has been updated
-    const updatedToggleButtons = wrapper.find(EuiButtonGroup);
-    expect(updatedToggleButtons.prop('idSelected')).toBe('span_list');
-  });
-
   it('handles user-defined zoom range via mini-map', () => {
     const wrapper = mount(<SpanDetailPanel {...mockProps} />);
 

diff --git a/public/components/trace_analytics/components/traces/span_detail_table.tsx b/public/components/trace_analytics/components/traces/span_detail_table.tsx
@@ -459,6 +459,7 @@ export function SpanDetailTableHierarchy(props: SpanDetailTableProps) {
       key="expandAll"
       color="text"
       iconType="expand"
+      data-test-subj="treeExpandAll"
     >
       Expand all
     </EuiButtonEmpty>,
@@ -468,6 +469,7 @@ export function SpanDetailTableHierarchy(props: SpanDetailTableProps) {
       key="collapseAll"
       color="text"
       iconType="minimize"
+      data-test-subj="treeCollapseAll"
     >
       Collapse all
     </EuiButtonEmpty>,

diff --git a/public/components/trace_analytics/components/traces/traces_content.tsx b/public/components/trace_analytics/components/traces/traces_content.tsx
@@ -73,10 +73,6 @@ export function TracesContent(props: TracesProps) {
   const [includeMetrics, setIncludeMetrics] = useState(false);
   const isNavGroupEnabled = coreRefs?.chrome?.navGroup.getNavGroupEnabled();
 
-  useEffect(() => {
-    refresh();
-  }, [startTime, endTime, props.dataSourceMDSId]);
-
   useEffect(() => {
     chrome.setBreadcrumbs([
       ...(isNavGroupEnabled ? [] : [props.parentBreadcrumb]),
@@ -119,6 +115,9 @@ export function TracesContent(props: TracesProps) {
     includeMetrics,
     tracesTableMode,
     props.setDataSourceMenuSelectable,
+    startTime,
+    endTime,
+    props.dataSourceMDSId,
   ]);
 
   const onToggle = (isOpen: boolean) => {

diff --git a/public/components/trace_analytics/index.scss b/public/components/trace_analytics/index.scss
@@ -122,3 +122,27 @@ th[data-test-subj^='tableHeaderCell_dashboard_latency_variance'] {
 .popOverSelectableItem {
   white-space: initial !important;
 }
+
+.full-screen-wrapper {
+  position: fixed;
+  top: 0;
+  left: 0;
+  width: 100vw;
+  height: 100vh;
+  background-color: #fff;
+  z-index: 9999;
+  display: flex;
+  flex-direction: column;
+}
+
+.full-screen-close-icon {
+  position: absolute;
+  top: 4px;
+  right: 4px;
+  z-index: 10000;
+}
+
+.full-screen-content {
+  flex: 1 1 auto;
+  overflow: auto;
+}
diff --git a/public/components/visualizations/plotly/plot.tsx b/public/components/visualizations/plotly/plot.tsx
@@ -16,7 +16,7 @@ interface PltProps {
   onUnhoverHandler?: (event: Readonly<Plotly.PlotMouseEvent>) => void;
   onClickHandler?: (event: Readonly<Plotly.PlotMouseEvent>) => void;
   onSelectedHandler?: (event: Readonly<Plotly.PlotSelectionEvent>) => void;
-  onRelayout?: (event: any) => void;
+  onRelayout?: (event: Readonly<Plotly.PlotSelectionEvent>) => void;
   height?: string;
   dispatch?: (props: any) => void;
 }