Remove use of react-dimensions package in ScatterPlot component (#1223

) ## Which problem is this PR solving? - Split from #1212 ## Short description of the changes This package is abandoned and doesn't play well with Vite, because one of its dependencies uses `this` to access the global context in a way that apparently fails in a module context. Replace this dependency with a simple hooks-based implementation. Signed-off-by: Máté Szabó <mszabo@fandom.com>
jaegertracing · Mar 1, 2023 · 49f326b · 49f326b
1 parent 9c6546d
commit 49f326b
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 52 deletions.
diff --git a/packages/jaeger-ui/package.json b/packages/jaeger-ui/package.json
@@ -88,7 +88,6 @@
     "raven-js": "^3.22.1",
     "react": "^18.2.0",
     "react-circular-progressbar": "^2.1.0",
-    "react-dimensions": "^1.3.1",
     "react-dom": "^18.2.0",
     "react-ga": "^3.3.1",
     "react-helmet": "^6.1.0",

diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.jsx b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.jsx
@@ -12,10 +12,9 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-import React from 'react';
+import React, { useRef, useState, useLayoutEffect } from 'react';
 import moment from 'moment';
 import PropTypes from 'prop-types';
-import dimensions from 'react-dimensions';
 import { XYPlot, XAxis, YAxis, MarkSeries, Hint } from 'react-vis';
 import { compose, withState, withProps } from 'recompose';
 
@@ -26,37 +25,58 @@ import './react-vis.css';
 import './ScatterPlot.css';
 
 function ScatterPlotImpl(props) {
-  const { data, containerWidth, onValueClick, overValue, onValueOver, onValueOut } = props;
+  const { data, onValueClick, overValue, onValueOver, onValueOut, calculateContainerWidth } = props;
+
+  const containerRef = useRef(null);
+  const [containerWidth, setContainerWidth] = useState(0);
+
+  useLayoutEffect(() => {
+    function updateContainerWidth() {
+      if (containerRef.current) {
+        setContainerWidth(calculateContainerWidth(containerRef.current));
+      }
+    }
+
+    // Calculate the initial width on first render.
+    updateContainerWidth();
+
+    window.addEventListener('resize', updateContainerWidth);
+
+    return () => window.removeEventListener('resize', updateContainerWidth);
+  }, []);
+
   return (
-    <div className="TraceResultsScatterPlot">
-      <XYPlot
-        margin={{
-          left: 50,
-        }}
-        width={containerWidth}
-        colorType="literal"
-        height={200}
-      >
-        <XAxis
-          title="Time"
-          tickTotal={4}
-          tickFormat={t => moment(t / ONE_MILLISECOND).format('hh:mm:ss a')}
-        />
-        <YAxis title="Duration" tickTotal={3} tickFormat={t => formatDuration(t)} />
-        <MarkSeries
-          sizeRange={[3, 10]}
-          opacity={0.5}
-          onValueClick={onValueClick}
-          onValueMouseOver={onValueOver}
-          onValueMouseOut={onValueOut}
-          data={data}
-        />
-        {overValue && (
-          <Hint value={overValue}>
-            <h4 className="scatter-plot-hint">{overValue.name || FALLBACK_TRACE_NAME}</h4>
-          </Hint>
-        )}
-      </XYPlot>
+    <div className="TraceResultsScatterPlot" ref={containerRef}>
+      {containerWidth && (
+        <XYPlot
+          margin={{
+            left: 50,
+          }}
+          width={containerWidth}
+          colorType="literal"
+          height={200}
+        >
+          <XAxis
+            title="Time"
+            tickTotal={4}
+            tickFormat={t => moment(t / ONE_MILLISECOND).format('hh:mm:ss a')}
+          />
+          <YAxis title="Duration" tickTotal={3} tickFormat={t => formatDuration(t)} />
+          <MarkSeries
+            sizeRange={[3, 10]}
+            opacity={0.5}
+            onValueClick={onValueClick}
+            onValueMouseOver={onValueOver}
+            onValueMouseOut={onValueOut}
+            data={data}
+          />
+          {overValue && (
+            <Hint value={overValue}>
+              <h4 className="scatter-plot-hint">{overValue.name || FALLBACK_TRACE_NAME}</h4>
+            </Hint>
+          )}
+        </XYPlot>
+      )}
     </div>
   );
 }
@@ -70,17 +90,18 @@ const valueShape = PropTypes.shape({
 });
 
 ScatterPlotImpl.propTypes = {
-  containerWidth: PropTypes.number,
   data: PropTypes.arrayOf(valueShape).isRequired,
   overValue: valueShape,
   onValueClick: PropTypes.func.isRequired,
   onValueOut: PropTypes.func.isRequired,
   onValueOver: PropTypes.func.isRequired,
+  calculateContainerWidth: PropTypes.func,
 };
 
 ScatterPlotImpl.defaultProps = {
-  containerWidth: null,
   overValue: null,
+  // JSDOM does not, as of 2023, have a layout engine, so allow tests to supply a mock width as a workaround.
+  calculateContainerWidth: container => container.clientWidth,
 };
 
 const ScatterPlot = compose(
@@ -93,4 +114,4 @@ const ScatterPlot = compose(
 
 export { ScatterPlotImpl };
 
-export default dimensions()(ScatterPlot);
+export default ScatterPlot;
diff --git a/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.test.js b/packages/jaeger-ui/src/components/SearchTracePage/SearchResults/ScatterPlot.test.js
@@ -14,7 +14,7 @@
 
 import React from 'react';
 import { mount, shallow } from 'enzyme';
-import { XAxis, YAxis } from 'react-vis';
+import { XAxis, XYPlot, YAxis } from 'react-vis';
 
 import ScatterPlot, { ScatterPlotImpl } from './ScatterPlot';
 import { ONE_MILLISECOND } from '../../../utils/date';
@@ -70,8 +70,7 @@ it('<ScatterPlot /> should render base case correctly', () => {
 it('<ScatterPlotImpl /> should render X axis correctly', () => {
   const wrapper = mount(
     <ScatterPlotImpl
-      containerWidth={1200}
-      containerHeight={200}
+      calculateContainerWidth={() => 1200}
       data={sampleData}
       onValueClick={() => null}
       onValueOut={() => null}
@@ -91,8 +90,7 @@ it('<ScatterPlotImpl /> should render X axis correctly', () => {
 it('<ScatterPlotImpl /> should render Y axis correctly', () => {
   const wrapper = mount(
     <ScatterPlotImpl
-      containerWidth={1200}
-      containerHeight={200}
+      calculateContainerWidth={() => 1200}
       data={sampleData}
       onValueClick={() => null}
       onValueOut={() => null}
@@ -107,3 +105,38 @@ it('<ScatterPlotImpl /> should render Y axis correctly', () => {
   expect(yAxisText).toContain('60ms');
   expect(yAxisText).toContain('80ms');
 });
+
+it('<ScatterPlotImpl /> should set fixed container width on initial render', () => {
+  const wrapper = mount(
+    <ScatterPlotImpl
+      calculateContainerWidth={() => 1200}
+      data={sampleData}
+      onValueClick={() => null}
+      onValueOut={() => null}
+      onValueOver={() => null}
+    />
+  );
+
+  const xyPlot = wrapper.find(XYPlot).getDOMNode();
+
+  expect(xyPlot.style.width).toBe('1200px');
+});
+
+it('<ScatterPlotImpl /> should update container width on window resize', () => {
+  const calculateContainerWidth = jest.fn().mockReturnValue(1200).mockReturnValue(700);
+  const wrapper = mount(
+    <ScatterPlotImpl
+      calculateContainerWidth={calculateContainerWidth}
+      data={sampleData}
+      onValueClick={() => null}
+      onValueOut={() => null}
+      onValueOver={() => null}
+    />
+  );
+
+  window.dispatchEvent(new Event('resize'));
+
+  const xyPlot = wrapper.find(XYPlot).getDOMNode();
+
+  expect(xyPlot.style.width).toBe('700px');
+});
diff --git a/yarn.lock b/yarn.lock
@@ -6551,10 +6551,6 @@ electron-to-chromium@^1.4.251:
   resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.284.tgz#61046d1e4cab3a25238f6bf7413795270f125592"
   integrity sha512-M8WEXFuKXMYMVr45fo8mq0wUrrJHheiKZf6BArTKk9ZBYCKJEOU5H8cdWgDT+qCVZf7Na4lVUaZsA+h6uA9+PA==
 
-element-resize-event@^2.0.4:
-  version "2.0.9"
-  resolved "https://registry.yarnpkg.com/element-resize-event/-/element-resize-event-2.0.9.tgz#2f5e1581a296eb5275210c141bc56342e218f876"
-
 elliptic@^6.0.0:
   version "6.5.4"
   resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.4.tgz#da37cebd31e79a1367e941b592ed1fbebd58abbb"
@@ -14742,13 +14738,6 @@ react-dev-utils@^7.0.1:
     strip-ansi "5.0.0"
     text-table "0.2.0"
 
-react-dimensions@^1.3.1:
-  version "1.3.1"
-  resolved "https://registry.yarnpkg.com/react-dimensions/-/react-dimensions-1.3.1.tgz#89c29bcd48828a74faeb07da1e461e1a354ccc48"
-  integrity sha512-go5vMuGUxaB5PiTSIk+ZfAxLbHwcIgIfLhkBZ2SIMQjaCgnpttxa30z5ijEzfDjeOCTGRpxvkzcmE4Vt4Ppvyw==
-  dependencies:
-    element-resize-event "^2.0.4"
-
 react-dom@^18.2.0:
   version "18.2.0"
   resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-18.2.0.tgz#22aaf38708db2674ed9ada224ca4aa708d821e3d"