Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions packages/chart/src/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
ModeBarButtonAny,
} from 'plotly.js';
import type { PlotParams } from 'react-plotly.js';
import { bindAllMethods } from '@deephaven/utils';
import { bindAllMethods, mergeRefs } from '@deephaven/utils';
import createPlotlyComponent from './plotly/createPlotlyComponent';
import Plotly from './plotly/Plotly';
import ChartModel from './ChartModel';
Expand Down Expand Up @@ -57,7 +57,7 @@ interface ChartProps {

isActive: boolean;
Plotly: typeof Plotly;
containerRef?: React.RefObject<HTMLDivElement>;
containerRef?: React.Ref<HTMLDivElement>;
onDisconnect: () => void;
onReconnect: () => void;
onUpdate: (obj: { isLoading: boolean }) => void;
Expand Down Expand Up @@ -156,7 +156,8 @@ class Chart extends Component<ChartProps, ChartState> {

this.PlotComponent = createPlotlyComponent(props.Plotly);
this.plot = React.createRef();
this.plotWrapper = props.containerRef ?? React.createRef();
this.plotWrapper = React.createRef();
this.plotWrapperMerged = mergeRefs(this.plotWrapper, props.containerRef);
this.columnFormats = [];
this.dateTimeFormatterOptions = {};
this.decimalFormatOptions = {};
Expand Down Expand Up @@ -238,6 +239,8 @@ class Chart extends Component<ChartProps, ChartState> {

plotWrapper: RefObject<HTMLDivElement>;

plotWrapperMerged: React.RefCallback<HTMLDivElement>;

columnFormats?: FormattingRule[];

dateTimeFormatterOptions?: DateTimeColumnFormatterOptions;
Expand Down Expand Up @@ -713,7 +716,7 @@ class Chart extends Component<ChartProps, ChartState> {
const isPlotShown = data != null;

return (
<div className="h-100 w-100 chart-wrapper" ref={this.plotWrapper}>
<div className="h-100 w-100 chart-wrapper" ref={this.plotWrapperMerged}>
{isPlotShown && (
<PlotComponent
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
Expand Down
7 changes: 5 additions & 2 deletions packages/dashboard-core-plugins/src/panels/ChartPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,11 @@ interface OwnProps extends DashboardPanelProps {
makeModel: () => Promise<ChartModel>;
localDashboardId: string;
Plotly?: typeof PlotlyType;
/** The plot container div */
containerRef?: RefObject<HTMLDivElement>;
/**
* The plot container div.
* The ref will be undefined on initial render if the chart needs to be loaded.
*/
containerRef?: React.Ref<HTMLDivElement>;

panelState?: GLChartPanelState;
}
Expand Down
1 change: 1 addition & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,4 @@ export { default as ValidationError } from './ValidationError';
export { default as TestUtils } from './TestUtils';
export * from './TestUtils';
export * from './UIConstants';
export * from './mergeRefs';
41 changes: 41 additions & 0 deletions packages/utils/src/mergeRefs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import React from 'react';
import { mergeRefs } from './mergeRefs';

describe('mergeRefs', () => {
test('merges ref objects', () => {
const refA = React.createRef();
const refB = React.createRef();
const mergedRef = mergeRefs(refA, refB);

const refValue = {};
mergedRef(refValue);
expect(refA.current).toBe(refValue);
expect(refB.current).toBe(refValue);
});

test('merges ref callbacks', () => {
const refA: React.RefCallback<unknown> = jest.fn();
const refB: React.RefCallback<unknown> = jest.fn();
const mergedRef = mergeRefs(refA, refB);

const refValue = {};
mergedRef(refValue);
expect(refA).toHaveBeenCalledWith(refValue);
expect(refB).toHaveBeenCalledWith(refValue);
});

test('ignores null/undefined refs', () => {
const refA = React.createRef();
const refB: React.RefCallback<unknown> = jest.fn();
const refC = null;
const refD = undefined;
const mergedRef = mergeRefs(refA, refB, refC, refD);

const refValue = {};
mergedRef(refValue);
expect(refA.current).toBe(refValue);
expect(refB).toHaveBeenCalledWith(refValue);
expect(refC).toBe(null);
expect(refD).toBe(undefined);
});
});
31 changes: 31 additions & 0 deletions packages/utils/src/mergeRefs.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type React from 'react';
Comment thread
mattrunyon marked this conversation as resolved.
Outdated

/**
* Merge multiple react refs into a single ref callback.
* This can be used to merge callback and object refs into a single ref.
* Merged callback refs will be called while object refs will have their current property set.
* @param refs The refs to merge
* @returns A ref callback that will set the value on all refs
*/
export function mergeRefs<T = unknown>(
...refs: Array<
React.MutableRefObject<T> | React.LegacyRef<T> | null | undefined
>
): React.RefCallback<T> {
return value => {
refs.forEach(ref => {
if (ref != null) {
if (typeof ref === 'function') {
ref(value);
} else {
// React marks RefObject as readonly, but it's just to indicate React manages it
// We can still write to its current value
// eslint-disable-next-line no-param-reassign
(ref as React.MutableRefObject<T | null>).current = value;
}
}
});
};
}

export default mergeRefs;