diff --git a/src/vis/EagerVis.tsx b/src/vis/EagerVis.tsx index 9ca021797..5c0894970 100644 --- a/src/vis/EagerVis.tsx +++ b/src/vis/EagerVis.tsx @@ -204,76 +204,40 @@ export function EagerVis({ const { getVisByType } = useVisProvider(); - // Each time you switch between vis config types, there is one render where the config is inconsistent with the type before the merge functions in the useEffect below can be called. - // To ensure that we never render an incosistent config, keep a consistent and a current in the config. Always render the consistent. // eslint-disable-next-line @typescript-eslint/naming-convention - const [{ consistent: visConfig, current: inconsistentVisConfig }, _setVisConfig] = React.useState<{ - consistent: BaseVisConfig; - current: BaseVisConfig; - }>( - externalConfig - ? { consistent: null, current: externalConfig } + const [_visConfig, _setVisConfig] = useUncontrolled({ + value: externalConfig?.type ? getVisByType(externalConfig?.type)?.mergeConfig(columns, externalConfig) : null, + defaultValue: externalConfig + ? null : columns.filter((c) => c.type === EColumnTypes.NUMERICAL).length > 1 - ? { - consistent: null, - current: { - type: ESupportedPlotlyVis.SCATTER, - numColumnsSelected: [], - color: null, - numColorScaleType: ENumericalColorScaleType.SEQUENTIAL, - shape: null, - dragMode: EScatterSelectSettings.RECTANGLE, - alphaSliderVal: 0.5, - } as BaseVisConfig, - } - : { - consistent: null, - current: { - type: ESupportedPlotlyVis.BAR, - multiples: null, - group: null, - direction: EBarDirection.HORIZONTAL, - display: EBarDisplayType.ABSOLUTE, - groupType: EBarGroupingType.STACK, - numColumnsSelected: [], - catColumnSelected: null, - aggregateColumn: null, - aggregateType: EAggregateTypes.COUNT, - } as BaseVisConfig, - }, - ); - - const setExternalConfigRef = useSyncedRef(setExternalConfig); - useEffect(() => { - setExternalConfigRef.current?.(visConfig); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [JSON.stringify(visConfig), setExternalConfigRef]); - - const setVisConfig = React.useCallback((newConfig: BaseVisConfig) => { - _setVisConfig((oldConfig) => { - return { - current: newConfig, - consistent: oldConfig.current.type !== newConfig.type ? oldConfig.consistent : newConfig, - }; - }); - }, []); - - React.useEffect(() => { - const vis = getVisByType(inconsistentVisConfig?.type); - if (vis) { - const newConfig = vis.mergeConfig(columns, inconsistentVisConfig); - _setVisConfig({ current: newConfig, consistent: newConfig }); - } - - // DANGER:: this useEffect should only occur when the visConfig.type changes. adding visconfig into the dep array will cause an infinite loop. - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [inconsistentVisConfig?.type, getVisByType]); + ? ({ + type: ESupportedPlotlyVis.SCATTER, + numColumnsSelected: [], + color: null, + numColorScaleType: ENumericalColorScaleType.SEQUENTIAL, + shape: null, + dragMode: EScatterSelectSettings.RECTANGLE, + alphaSliderVal: 0.5, + } as BaseVisConfig) + : ({ + type: ESupportedPlotlyVis.BAR, + multiples: null, + group: null, + direction: EBarDirection.HORIZONTAL, + display: EBarDisplayType.ABSOLUTE, + groupType: EBarGroupingType.STACK, + numColumnsSelected: [], + catColumnSelected: null, + aggregateColumn: null, + aggregateType: EAggregateTypes.COUNT, + } as BaseVisConfig), + onChange: setExternalConfig, + }); - useEffect(() => { - if (externalConfig) { - setVisConfig(externalConfig); - } - }, [externalConfig, setVisConfig]); + const setVisConfig = (v: BaseVisConfig) => { + const withDefaults = getVisByType(v.type)?.mergeConfig(columns, v); + _setVisConfig(withDefaults); + }; // Converting the selected list into a map, since searching through the list to find an item is common in the vis components. const selectedMap: { [key: string]: boolean } = useMemo(() => { @@ -309,17 +273,17 @@ export function EagerVis({ }; }, [colors]); - if (!visConfig) { - return
; - } - const commonProps = { showSidebar, setShowSidebar, enableSidebar, }; - const Renderer = getVisByType(visConfig?.type)?.renderer; + const Renderer = getVisByType(_visConfig?.type)?.renderer; + + if (!_visConfig || !Renderer) { + return null; + } return ( setShowSidebar(!showSidebar)} /> : null} - {Renderer ? ( - - ) : null} + {showSidebar ? ( - setShowSidebar(false)}> - + setShowSidebar(false)}> + ) : null} diff --git a/src/vis/heatmap/HeatmapGrid.tsx b/src/vis/heatmap/HeatmapGrid.tsx index 40872f2b6..2a5eed012 100644 --- a/src/vis/heatmap/HeatmapGrid.tsx +++ b/src/vis/heatmap/HeatmapGrid.tsx @@ -34,11 +34,8 @@ export function HeatmapGrid({ return ( - {status === 'pending' ? ( - - ) : !hasAtLeast2CatCols ? ( - - ) : ( + {status === 'pending' && } + {status === 'success' && hasAtLeast2CatCols && ( )} + {status === 'success' && !hasAtLeast2CatCols && ( + + )} ); } diff --git a/src/vis/heatmap/HeatmapText.tsx b/src/vis/heatmap/HeatmapText.tsx index bfaebadad..182cb7e28 100644 --- a/src/vis/heatmap/HeatmapText.tsx +++ b/src/vis/heatmap/HeatmapText.tsx @@ -26,7 +26,7 @@ export function HeatmapText({ isImmediate: boolean; }) { const labelSpacing = useMemo(() => { - const maxLabelLength = d3.max(yScale.domain().map((m) => m.length)); + const maxLabelLength = d3.max(yScale.domain().map((m) => m?.length)); return maxLabelLength > 5 ? 35 : maxLabelLength * 7; }, [yScale]);