From 9a6e9b28ffd5b5b16e8461a3dbb6feb677e37799 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Wed, 5 Aug 2026 18:23:57 -0700 Subject: [PATCH 01/11] charts grouping option --- src/firefly/js/charts/ChartUtil.js | 320 ++++++++++++++++-- src/firefly/js/charts/ChartsCntlr.js | 7 +- .../js/charts/dataTypes/FireflyGenericData.js | 7 +- .../js/charts/dataTypes/FireflySpectrum.js | 13 +- src/firefly/js/charts/ui/ChartSelectPanel.jsx | 40 ++- src/firefly/js/charts/ui/CombineChart.jsx | 24 +- src/firefly/js/charts/ui/PlotlyChartArea.jsx | 6 +- .../js/charts/ui/options/BasicOptions.jsx | 10 +- .../js/charts/ui/options/ScatterOptions.jsx | 95 +++++- .../js/charts/ui/options/SpectralLines.jsx | 15 +- .../js/charts/ui/options/SpectrumOptions.jsx | 98 ++++-- 11 files changed, 522 insertions(+), 113 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index b7996bc403..5364223828 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -16,8 +16,8 @@ import shallowequal from 'shallowequal'; import {getAppOptions} from '../core/AppDataCntlr.js'; import { - COL_TYPE, getColumnType, getMetaEntry, getTblById, isColumnType, isFullyLoaded, isTableLoaded, - stripColumnNameQuotes, SYS_COLUMNS, watchTableChanges + COL_TYPE, ensureEnumVals, getColumn, getColumns, getColumnType, getMetaEntry, getTblById, isColumnType, + isFullyLoaded, isTableLoaded, splitVals, stripColumnNameQuotes, SYS_COLUMNS, watchTableChanges } from '../tables/TableUtil.js'; import {TABLE_HIGHLIGHT, TABLE_LOADED, TABLE_SELECT, TABLE_SORT} from '../tables/TablesCntlr.js'; import {getSpectrumDM} from '../voAnalyzer/SpectrumDM.js'; @@ -37,7 +37,9 @@ import {SelectInfo} from '../tables/SelectInfo.js'; import {getTraceTSEntries as histogramTSGetter} from './dataTypes/FireflyHistogram.js'; import {getTraceTSEntries as heatmapTSGetter} from './dataTypes/FireflyHeatmap.js'; import {getTraceTSEntries as genericTSGetter} from './dataTypes/FireflyGenericData.js'; -import {getTraceTSEntries as spectrumTSGetter, spectrumPlot, spectrumType} from './dataTypes/FireflySpectrum.js'; +import { + getTraceTSEntries as spectrumTSGetter, spectrumPlot, spectrumType +} from './dataTypes/FireflySpectrum.js'; import {toRGBA as colorToRGBA} from '../util/Color.js'; import {MetaConst} from '../data/MetaConst'; import {ALL_COLORSCALE_NAMES, colorscaleNameToVal} from './Colorscale.js'; @@ -68,6 +70,22 @@ export const HIGHLIGHTED_PROPS = { const FSIZE = 12; export const TBL_SRC_PATTERN = /^tables::(.+)/; +export const GROUP_BY_NONE = ''; + +/** + * Extract table mappings from flattened chart data. + * @param {Object} flattenData flattened trace data + * @returns {Object} mappings keyed by trace data paths + */ +export function getTableSourceMappings(flattenData={}) { + return Object.entries(flattenData) + .filter(([, value]) => typeof value === 'string' && value.startsWith('tables::')) + .reduce((mappings, [key, value]) => { + const [, colExp] = value.match(TBL_SRC_PATTERN) || []; + if (colExp) mappings[key] = colExp; + return mappings; + }, {}); +} /** * Does the application only support single trace @@ -322,6 +340,244 @@ export function combineAllTraceFrom(chartId, selIndexes, newTraceProps) { } } +//Chart "Group By" logic helpers below +export function getGroupByColumn(chartId) { + const {fireflyData=[]} = getChartData(chartId) || {}; + return fireflyData.find((fd) => fd?.groupBy?.column)?.groupBy.column || GROUP_BY_NONE; +} + +export function isGroupedChart(chartId) { + return Boolean(getGroupByColumn(chartId)); +} + +export function isGroupByAllowed(chartId, activeTrace) { + const {data=[]} = getChartData(chartId) || {}; + if (!isUndefined(activeTrace) && activeTrace >= data.length) return false; + return Boolean(chartId) && (data.length <= 1 || isGroupedChart(chartId)); +} + +export function getGroupableColumns(tbl_id) { + const tableModel = getTblById(tbl_id); + if (!tableModel?.tableData?.columns) return []; + + const tableCopy = simpleCloneDeep(tableModel); + ensureEnumVals(tableCopy); //cloning table model to get list of enum-like columns in tableCopy + + return getColumns(tableCopy) + .filter(({name, enumVals, visibility}) => enumVals && !SYS_COLUMNS.includes(name) && visibility !== 'hidden') + .map((c) => ({...c})); +} + +export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, groupByColumn=GROUP_BY_NONE, changes={}}) { + const chartData = simpleCloneDeep(getChartData(chartId, {})); + Object.entries(changes).forEach(([key, value]) => set(chartData, key, value)); + + const data = chartData.data || []; + const fireflyData = chartData.fireflyData || []; + const tablesources = chartData.tablesources || []; + const baseData = simpleCloneDeep(data[activeTrace] || {}); + const baseFirefly = simpleCloneDeep(fireflyData[activeTrace] || {}); + const baseTable = getCanonicalTableInfo({ + data, fireflyData, tablesources, original: chartData._original, activeTrace, tbl_id + }); + + applyCanonicalTableMappings(baseData, baseFirefly, baseTable); + restoreGroupByBaseStyle(baseData, baseFirefly); + + const commonChanges = { + showOptions: false, + highlighted: undefined, + selected: undefined, + selection: undefined, + hasSelected: false, + tablesources: [] + }; + + if (!groupByColumn) { + clearGroupBy(baseData, baseFirefly); + return { + ...commonChanges, + data: [baseData], + fireflyData: [baseFirefly], + activeTrace: 0, + curveNumberMap: [0], + 'layout.showlegend': undefined, + 'layout.legend.title.text': undefined + }; + } + + const groupCol = getGroupableColumns(tbl_id).find((c) => c.name === groupByColumn) || getColumn(getTblById(tbl_id), groupByColumn); + const groupValues = splitVals(groupCol?.enumVals) + .map((v) => String(v ?? '').trim().replace(/^'(.*)'$/, '$1')) + .filter((v) => v !== ''); + if (!groupCol || groupValues.length === 0) { + clearGroupBy(baseData, baseFirefly); + return { + ...commonChanges, + data: [baseData], + fireflyData: [baseFirefly], + activeTrace: 0, + curveNumberMap: [0], + 'layout.showlegend': undefined, + 'layout.legend.title.text': undefined + }; + } + + const groupData = []; + const groupFireflyData = []; + groupValues.forEach((value, idx) => { + const trace = simpleCloneDeep(baseData); + const fd = simpleCloneDeep(baseFirefly); + const color = toRGBA(TRACE_COLORS[idx % TRACE_COLORS.length]); + + // Keep every generated trace independently connected to the original table. + // The table-source watcher can then rebuild any trace without depending on + // whichever generated trace happens to be active. + applyCanonicalTableMappings(trace, fd, baseTable); + + set(trace, 'name', value); + set(trace, 'showlegend', true); + set(trace, 'marker.color', color); + set(trace, 'line.color', color); + set(trace, 'error_x.color', color); + set(trace, 'error_y.color', color); + set(trace, 'legendgroup', uniqueId('grp')); + + fd.groupBy = { + column: groupCol.name, + value, + baseTraceStyle: getGroupByBaseStyle(baseData), + baseTable + }; + fd.filters = makeGroupFilter(groupCol.name, value); + + groupData.push(trace); + groupFireflyData.push(fd); + }); + + return { + ...commonChanges, + data: groupData, + fireflyData: groupFireflyData, + activeTrace: Math.min(activeTrace, groupData.length - 1), + curveNumberMap: range(groupData.length), + 'layout.showlegend': true, + 'layout.legend.title.text': groupCol.label || groupCol.name + }; +} + +function makeGroupFilter(columnName, value) { + const escapedColumn = columnName.replace(/"/g, '""'); + const escapedValue = String(value).replace(/'/g, "''"); + return `"${escapedColumn}" = '${escapedValue}'`; +} + +function clearGroupBy(trace, fireflyTrace) { + restoreGroupByBaseStyle(trace, fireflyTrace); + if (fireflyTrace?.groupBy?.baseTable) { + // Keep the canonical source available after collapsing generated groups. + // The next regroup must not depend on a transient tablesource watcher. + fireflyTrace.baseTable = fireflyTrace.groupBy.baseTable; + } + if (fireflyTrace?.groupBy) { + delete trace.name; + } + if (fireflyTrace) { + delete fireflyTrace.groupBy; + delete fireflyTrace.filters; + } +} + +function getGroupByBaseStyle(trace) { + return ['marker.color', 'line.color', 'error_x.color', 'error_y.color'] + .reduce((style, path) => { + if (has(trace, path)) set(style, path, get(trace, path)); + return style; + }, {}); +} + +function restoreGroupByBaseStyle(trace, fireflyTrace) { + const baseTraceStyle = fireflyTrace?.groupBy?.baseTraceStyle; + if (!baseTraceStyle) return; + Object.entries(flattenObject(baseTraceStyle)).forEach(([path, value]) => set(trace, path, value)); +} + +function normalizeTableMappings(mappings={}, traceNum) { + const normalized = {data: {}, fireflyData: {}}; + Object.entries(mappings).forEach(([key, value]) => { + if (key.startsWith(`fireflyData.${traceNum}.`)) { + normalized.fireflyData[key.replace(`fireflyData.${traceNum}.`, '')] = value; + } else { + normalized.data[key] = value; + } + }); + return normalized; +} + +function extractTableMappings(trace={}, fireflyTrace={}, traceNum) { + const mappings = {data: {}, fireflyData: {}}; + const flattenData = assign( + flattenObject(trace), + flattenObject(fireflyTrace, `fireflyData.${traceNum}`) + ); + Object.entries(getTableSourceMappings(flattenData)).forEach(([key, mapping]) => { + if (key.startsWith(`fireflyData.${traceNum}.`)) { + mappings.fireflyData[key.replace(`fireflyData.${traceNum}.`, '')] = mapping; + } else { + mappings.data[key] = mapping; + } + }); + return mappings; +} + +function mergeTableMappings(base={}, current={}) { + return { + data: {...(base.data || {}), ...(current.data || {})}, + fireflyData: {...(base.fireflyData || {}), ...(current.fireflyData || {})} + }; +} + +function getCanonicalTableInfo({data=[], fireflyData=[], tablesources=[], original={}, activeTrace=0, tbl_id}) { + const groupedBase = fireflyData.find((fd) => fd?.groupBy?.baseTable)?.groupBy?.baseTable || + fireflyData.find((fd) => fd?.baseTable)?.baseTable; + const activeSource = tablesources[activeTrace]; + const source = activeSource?.mappings ? activeSource : undefined; + const sourceMappings = normalizeTableMappings(source?.mappings, activeTrace); + const currentMappings = extractTableMappings(data[activeTrace], fireflyData[activeTrace], activeTrace); + const originalTraceNum = original?.data?.[activeTrace] ? activeTrace : 0; + const originalMappings = extractTableMappings( + original?.data?.[originalTraceNum], original?.fireflyData?.[originalTraceNum], activeTrace + ); + const base = groupedBase || {}; + + //Merge fallback mappings first and current trace/source mappings last + //later entries override earlier values while table sources rebuild + const mappingSources = [ + originalMappings, + base.mappings, + sourceMappings, + currentMappings + ]; + const mappings = mappingSources.reduce( + (merged, source) => mergeTableMappings(merged, source), {} + ); + + return { + tbl_id: base.tbl_id || source?.tbl_id || original?.data?.[originalTraceNum]?.tbl_id || tbl_id, + mappings + }; +} + +function applyCanonicalTableMappings(trace={}, fireflyTrace={}, tableInfo={}) { + if (tableInfo.tbl_id) set(trace, 'tbl_id', tableInfo.tbl_id); + Object.entries(tableInfo.mappings?.data || {}).forEach(([key, value]) => { + if (value !== undefined) set(trace, key, `tables::${value}`); + }); + Object.entries(tableInfo.mappings?.fireflyData || {}).forEach(([key, value]) => { + if (value !== undefined) set(fireflyTrace, key, `tables::${value}`); + }); +} + export function newTraceFrom(data, selIndexes, newTraceProps, traceAnnotations) { const sdata = simpleCloneDeep(pick(data, ['x', 'y', 'z', 'legendgroup', 'error_x', 'error_y', 'text', 'hovertext', 'marker', 'hoverinfo', 'firefly' ])); @@ -627,7 +883,7 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { if (action.type === TABLE_HIGHLIGHT) { // ignore if traceNum is not active const {activeTrace=0} = getChartData(chartId); - if (traceNum !== activeTrace && !isSpectralOrder(chartId)) return; + if (traceNum !== activeTrace && !isGroupedChart(chartId)) return; const {highlightedRow} = action.payload; updateHighlighted(chartId, traceNum, highlightedRow); } else if (action.type === TABLE_SELECT) { @@ -660,11 +916,6 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { } } -export function isSpectralOrder(chartId) { - const {activeTrace=0, fireflyData} = getChartData(chartId) || {}; - return fireflyData?.[activeTrace]?.spectralOrder; -} - export function isSpectrum(chartId) { const {activeTrace=0, fireflyData} = getChartData(chartId) || {}; return fireflyData?.[activeTrace]?.dataType === spectrumType; @@ -673,16 +924,6 @@ export function isSpectrum(chartId) { function makeTableSources(chartId, data=[], fireflyData=[]) { - const convertToDS = (flattenData) => - Object.entries(flattenData) - .filter(([,v]) => typeof v === 'string' && v.startsWith('tables::')) - .reduce( (p, [k,v]) => { - const [,colExp] = v.match(TBL_SRC_PATTERN) || []; - if (colExp) set(p, ['mappings',k], colExp); - return p; - }, {}); - - // for some firefly specific chart types the data are const currentData = (data.length < fireflyData.length) ? fireflyData : data; @@ -690,7 +931,7 @@ function makeTableSources(chartId, data=[], fireflyData=[]) { const fireflyDataFlatten = flattenObject(fireflyData[traceNum] || {}, `fireflyData.${traceNum}`); // fireflyData mappings have full path const flattenData = assign(flattenObject(data[traceNum] || {}), fireflyDataFlatten); - const ds = data[traceNum] ? convertToDS(flattenData) : {}; //avoid flattening arrays + const ds = data[traceNum] ? {mappings: getTableSourceMappings(flattenData)} : {}; //avoid flattening arrays // table id can be a part of fireflyData too const tbl_id = get(data, `${traceNum}.tbl_id`) || get(fireflyData, `${traceNum}.tbl_id`); @@ -1202,10 +1443,39 @@ function hasNoXY(type, tablesource) { export function getChartProps(chartId, tbl_id, activeTrace) { const {data, layout, fireflyLayout, tablesources, fireflyData, ...rest} = getChartData(chartId) || {}; activeTrace = activeTrace ?? rest.activeTrace ?? 0; - const tablesource = get(tablesources, [activeTrace], tbl_id && {tbl_id}); - tbl_id = tbl_id || tablesource?.tbl_id; + const activeTablesource = get(tablesources, [activeTrace]); + + //Group changes temporarily reset table sources; recover table ID and mappings + //when grouped or fetched traces no longer retain them + const canonicalTable = getCanonicalTableInfo({ + data, + fireflyData, + tablesources, + original: rest._original, + activeTrace, + tbl_id: tbl_id + }); + tbl_id = tbl_id || activeTablesource?.tbl_id || canonicalTable?.tbl_id; + + const canonicalMappings = { + ...(canonicalTable?.mappings?.data || {}), + ...Object.entries(canonicalTable?.mappings?.fireflyData || {}) + .reduce((p, [key, value]) => set(p, `fireflyData.${activeTrace}.${key}`, value), {}) + }; + + + const activeMappings = Object.fromEntries( + Object.entries(activeTablesource?.mappings || {}) + .filter(([, value]) => value !== undefined && value !== '') + ); - const mappings = tablesource?.mappings; + //Prefer current/active non-empty mappings, falling back to canonical/original mappings + const mappings = {...canonicalMappings, ...activeMappings}; + + //Rebuild a usable table source while preserving existing source behavior + const tablesource = (activeTablesource || tbl_id) + ? {...(activeTablesource || {}), tbl_id, mappings} + : undefined; const multiTrace = activeTrace > 0 || data?.length > 1; const type = get(data, `${activeTrace}.type`, 'scatter'); const dataType = fireflyData?.[activeTrace]?.dataType; @@ -1250,9 +1520,9 @@ function simpleCloneDeep(obj) { const copy = {}; for (const key of Object.keys(obj)) { - if (obj.hasOwnProperty(key)) { + if (Object.prototype.hasOwnProperty.call(obj, key)) { copy[key] = simpleCloneDeep(obj[key]); } } return copy; -} \ No newline at end of file +} diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index 20f5f74f65..7cf56b329f 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -16,7 +16,7 @@ import {dispatchAddViewerItems, dispatchUpdateCustom, dispatchRemoveViewerItems, import {DEFAULT_PLOT2D_VIEWER_ID} from '../visualize/VisConst'; import {applyDefaults, flattenAnnotations, formatColExpr, getPointIdx, getRowIdx, handleTableSourceConnections, clearChartConn, newTraceFrom, setupTableWatcher, HIGHLIGHTED_PROPS, SELECTED_PROPS, TBL_SRC_PATTERN, - getSelIndexes, isSpectralOrder, combineAllTraceFrom} from './ChartUtil.js'; + getSelIndexes, isGroupedChart, combineAllTraceFrom} from './ChartUtil.js'; import {FilterInfo} from '../tables/FilterInfo.js'; import {SelectInfo} from '../tables/SelectInfo.js'; import {REINIT_APP} from '../core/CoreConst'; @@ -412,7 +412,7 @@ function chartSelect(action) { } else { let selected = undefined; const hasSelected = !isEmpty(selIndexes); - if (isSpectralOrder(chartId)) { + if (isGroupedChart(chartId)) { selected = combineAllTraceFrom(chartId, selIndexes, SELECTED_PROPS); dispatchChartUpdate({chartId, changes: {hasSelected, selected, selection: undefined}}); } else { @@ -493,7 +493,7 @@ function getActiveTraceChanges({chartId, activeTrace, data, fireflyData, tableso const selectInfoCls = SelectInfo.newInstance(selectInfo); const selIndexes = getSelIndexes(data, selectInfoCls, activeTrace); if (selIndexes.length > 0) { - if (isSpectralOrder(chartId)) { + if (isGroupedChart(chartId)) { selected = combineAllTraceFrom(chartId, selIndexes, SELECTED_PROPS); } else { const traceAnnotations = get(fireflyData, `${activeTrace}.annotations`); @@ -964,4 +964,3 @@ export function removeChartsInGroup(groupId) { .filter( (v) => !groupId || v.groupId === groupId) .forEach( (v) => dispatchChartRemove(v.chartId)); } - diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index c6d46ca7fa..77934593f4 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -2,7 +2,7 @@ * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ import {get, isArray, isUndefined, uniqueId, isNil} from 'lodash'; -import {getTblById, getColumns, getColumn, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; +import {getTblById, getColumns, getColumn, getColumnIdx, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; import {cloneRequest, makeSubQueryRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {dispatchChartUpdate, dispatchError, getChartData, getTraceSymbol, hasUpperLimits, hasLowerLimits} from '../ChartsCntlr.js'; import {formatColExpr, getDataChangesForMappings, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; @@ -87,6 +87,11 @@ async function fetchData(chartId, traceNum, tablesource) { // extra changes based on trace type addOtherChanges({changes, chartId, traceNum, tablesource, tableModel: originalTableModel}); + const origIdx = getColumnIdx(tableModel, 'ORIG_IDX'); + if (origIdx >= 0) { + changes[`data.${traceNum}.firefly.rowIdx`] = tableModel.tableData.data.map((row) => row[origIdx]); + } + dispatchChartUpdate({chartId, changes}); const {activeTrace} = getChartData(chartId); if (isUndefined(activeTrace) || activeTrace === traceNum) { diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index 9a0504a221..bffcbeb201 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -1,12 +1,12 @@ /* * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ -import {isEmpty, pickBy, cloneDeep, set} from 'lodash'; +import {isEmpty, pickBy, cloneDeep, set, uniqueId} from 'lodash'; import {getTblById, getColumn, doFetchTable, getColumnIdx} from '../../tables/TableUtil.js'; import {getSpectrumDM, REF_POS} from '../../voAnalyzer/SpectrumDM.js'; import {dispatchChartUpdate, dispatchError} from '../ChartsCntlr.js'; -import {getDataChangesForMappings, updateHighlighted, updateSelection, getMinScatterGLRows, isSpectralOrder} from '../ChartUtil.js'; +import {getDataChangesForMappings, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; import {addOtherChanges, createChartTblRequest, getTraceTSEntries as genericTSGetter} from './FireflyGenericData.js'; import {quoteNonAlphanumeric} from '../../util/expr/Variable.js'; @@ -27,7 +27,7 @@ export function getTraceTSEntries({traceTS, chartId, traceNum}) { const traceEntry = genericTSGetter({traceTS, chartId, traceNum}); if (isEmpty(traceEntry)) return {}; - if (!isSpectralOrder(chartId)) { + if (!isGroupedChart(chartId)) { return {options: traceEntry.options, fetchData:traceEntry.fetchData}; } return {options: traceEntry.options, fetchData}; @@ -110,7 +110,8 @@ export function spectrumPlot({tbl_id, spectrumDM}) { const order = cloneDeep(orig); set(order, 'name', v); set(order, 'mode', 'lines+markers'); - set(order, 'firefly.spectralOrder', spectralAxis.order); + set(order, 'legendgroup', uniqueId('grp')); + set(order, 'firefly.groupBy', {column: spectralAxis.order, value: v}); set(order, 'firefly.filters', `"${orderCol.name}" = '${v}'`); data.push(order); }); @@ -156,7 +157,7 @@ export function getSpectrumProps(tbl_id, spectrumDM) { // get default spectral frame and labels, needed to initialize spectrum // (in future, move redshift processing functions & defaults from SpectrumOptions to a separate file where they can be exported to avoid redundancy) - const refPos = spectralFrame.refPos.toUpperCase(); + const refPos = spectralFrame?.refPos?.toUpperCase?.() || REF_POS.TOPOCENTER; //add a fallback to TOPOCENTER if refPos is undefined const sfLabel = refPos===REF_POS.TOPOCENTER ? 'Observed Frame' : refPos===REF_POS.CUSTOM ? 'Rest Frame' : `${refPos} Spectral Frame`; const redshiftLabel = refPos===REF_POS.CUSTOM ? `Custom Redshift = ${spectralFrame.redshift}` : ''; @@ -168,4 +169,4 @@ export function getSpectrumProps(tbl_id, spectrumDM) { return {spectralAxis, fluxAxis, mode, x, y, xErrArray, xErrArrayMinus, yErrArray, yErrArrayMinus, xMax, xMin, yMax, yMin, xUnit, yUnit, xLabel, yLabel, isSED, spectralFrame, derivedRedshift, target}; -} \ No newline at end of file +} diff --git a/src/firefly/js/charts/ui/ChartSelectPanel.jsx b/src/firefly/js/charts/ui/ChartSelectPanel.jsx index 676d6035c0..b74da246c6 100644 --- a/src/firefly/js/charts/ui/ChartSelectPanel.jsx +++ b/src/firefly/js/charts/ui/ChartSelectPanel.jsx @@ -10,7 +10,7 @@ import {useStoreConnector} from './../../ui/SimpleComponent.jsx'; import {getChartData, dispatchChartTraceRemove, dispatchChartUpdate} from '../ChartsCntlr.js'; import {NewTracePanel, getNewTraceType, getSubmitChangesFunc, addNewTrace} from './options/NewTracePanel.jsx'; import {PopupPanel} from './../../ui/PopupPanel.jsx'; -import {isSpectralOrder, isScatter2d, getTblIdFromChart} from '../ChartUtil.js'; +import {isGroupedChart, isScatter2d, getTblIdFromChart} from '../ChartUtil.js'; import {BasicOptions, useBasicOptions} from './options/BasicOptions.jsx'; import {ScatterOptions} from './options/ScatterOptions.jsx'; import {HeatmapOptions} from './options/HeatmapOptions.jsx'; @@ -34,13 +34,13 @@ function getChartActions({chartId, tbl_id}) { if (data.length > 0) { // can modify active trace chartActions.push(CHART_TRACE_MODIFY); - if (data.length > 1 && !isSpectralOrder(chartId)) { + if (data.length > 1 && !isGroupedChart(chartId)) { // can remove active trace chartActions.push(CHART_TRACE_REMOVE); } } if (tbl_id) { - if (!isSpectralOrder(chartId)) { + if (!isGroupedChart(chartId)) { // can add trace chartActions.push(CHART_TRACE_ADDNEW); } @@ -88,26 +88,32 @@ function onChartAction({chartAction, tbl_id, chartId, hideDialog, renderTreeId}) }; } -function getGroupKey(chartId, chartAction) { +function getGroupKey(chartId, chartAction, activeTrace) { if (chartAction === CHART_ADDNEW || chartAction === CHART_TRACE_ADDNEW) { const type = getNewTraceType(); const cid = (chartAction === CHART_ADDNEW) ? 'newchart' : chartId; return `${cid}-newtrace-${type}`; } else { - const {activeTrace} = getChartData(chartId); - return `${chartId}-${activeTrace}`; + const trace = activeTrace ?? getChartData(chartId)?.activeTrace ?? 0; + return `${chartId}-${trace}`; } } export function ChartSelectPanel({tbl_id, chartId, chartAction, inputStyle={}, hideDialog, sx={}}) { const {renderTreeId} = useContext(RenderTreeIdCtx); const showActionOptions= chartAction!==CHART_ADDNEW; + const isGrouped = useStoreConnector(() => isGroupedChart(chartId), [chartId]); + const activeTrace = useStoreConnector(() => getChartData(chartId)?.activeTrace ?? 0, [chartId]); - const chartActions = showActionOptions ? getChartActions({chartId, tbl_id}) : [CHART_ADDNEW]; + const chartActions = useStoreConnector(() => showActionOptions ? getChartActions({chartId, tbl_id}) : [CHART_ADDNEW], + [chartId, tbl_id, showActionOptions]); const [chartActionState, setChartActionState] = useState( (chartActions.includes(chartAction)) ? chartAction : chartActions[0]); + useEffect(() => { + setChartActionState((current) => chartActions.includes(current) ? current : chartActions[0]); + }, [chartActions]); - const groupKey = getGroupKey(chartId, chartActionState); + const groupKey = getGroupKey(chartId, chartActionState, activeTrace); const chartActionChanged = (chartAction) => setChartActionState(chartAction); @@ -126,9 +132,9 @@ export function ChartSelectPanel({tbl_id, chartId, chartAction, inputStyle={}, h }}> - {showActionOptions && + {showActionOptions && !isGrouped && } - {showActionOptions && } + {showActionOptions && !isGrouped && } @@ -187,7 +193,7 @@ function ChartAction({chartId, chartActions, chartAction, chartActionChanged}) { value={chartAction} onChange={onChartActionChange} /> - + {!isGroupedChart(chartId) && } ); } @@ -210,7 +216,7 @@ function ChartActionOptions(props) { return (); } if (chartAction === CHART_TRACE_MODIFY) { - return (); + return (); } else if (chartAction === CHART_TRACE_REMOVE) { const {data=[], activeTrace} = getChartData(chartId); const traceName = get(data, `${activeTrace}.name`) || `trace ${activeTrace}`; @@ -279,16 +285,20 @@ function SyncedOptionsUI (props) { SyncedOptionsUI.propTypes = { chartId: PropTypes.string, + tbl_id: PropTypes.string, groupKey: PropTypes.string }; /** * Creates and shows the modal dialog with chart options. * @param {string} chartId + * @param {string} chartAction + * @param {string} tbl_id */ export function showChartsDialog(chartId,chartAction, tbl_id) { - const {data, fireflyData, activeTrace} = getChartData(chartId); - const workingTblId = tbl_id ?? (get(data, `${activeTrace}.tbl_id`) || get(fireflyData, `${activeTrace}.tbl_id`)); + const {data, fireflyData, activeTrace, groupId} = getChartData(chartId); + const workingTblId = tbl_id ?? (get(data, `${activeTrace}.tbl_id`) || + get(fireflyData, `${activeTrace}.tbl_id`) || groupId); const popupId ='chartOptionsDialog'; const dialogContent= ( @@ -304,5 +314,3 @@ export function showChartsDialog(chartId,chartAction, tbl_id) { DialogRootContainer.defineDialog(popupId, dialogContent); dispatchShowDialog(popupId); } - - diff --git a/src/firefly/js/charts/ui/CombineChart.jsx b/src/firefly/js/charts/ui/CombineChart.jsx index 5083bc7349..1e76145e31 100644 --- a/src/firefly/js/charts/ui/CombineChart.jsx +++ b/src/firefly/js/charts/ui/CombineChart.jsx @@ -9,7 +9,7 @@ import {getMultiViewRoot, getViewerItemIds} from '../../visualize/MultiViewCntlr import {PINNED_CHART_VIEWER_ID} from '../../visualize/VisConst'; import {getSpectrumDM} from '../../voAnalyzer/SpectrumDM.js'; import {dispatchChartAdd, getChartData} from '../ChartsCntlr.js'; -import {getNewTraceDefaults, getTblIdFromChart, isSpectralOrder, uniqueChartId} from '../ChartUtil.js'; +import {getNewTraceDefaults, getTblIdFromChart, isGroupedChart, uniqueChartId} from '../ChartUtil.js'; import {PINNED_GROUP, PINNED_CHART_PREFIX} from './PinnedChartContainer.jsx'; import {useStoreConnector} from '../../ui/SimpleComponent.jsx'; import {FieldGroup} from '../../ui/FieldGroup.jsx'; @@ -74,7 +74,11 @@ export const CombineChart = ({chartIds, selectedChartId, showChartSelectionTable onCombineComplete?.(); // post-combination callback }; - return (chartIds?.length > 1) + const unresolvedChartIds = chartIds?.[0] instanceof Promise; + const hasChartsToCombine = chartIds?.length > 1 && + (!showChartSelectionTable || unresolvedChartIds || hasCompatibleCharts(chartIds, selectedChartId)); + + return hasChartsToCombine ? : null; }; @@ -111,8 +115,10 @@ CombineChart.propTypes = { export const CombinePinnedCharts = ({viewerId, slotProps}) => { if (viewerId !== PINNED_CHART_VIEWER_ID) return null; - const chartIds = getViewerItemIds(getMultiViewRoot(), viewerId); - const selectedChartId = getActiveViewerItemId(viewerId, true); + const {chartIds, selectedChartId} = useStoreConnector(() => ({ + chartIds: getViewerItemIds(getMultiViewRoot(), viewerId), + selectedChartId: getActiveViewerItemId(viewerId, true) + }), [viewerId]); return ; }; @@ -183,6 +189,12 @@ function createTableModel(chartIds, selectedChartId, showAll, tbl_id) { return table; } +export function hasCompatibleCharts(chartIds=[], selectedChartId) { + selectedChartId = selectedChartId ?? chartIds[0]; + if (!selectedChartId) return false; + return chartIds.some((id) => id !== selectedChartId && canCombine(id, selectedChartId)); +} + const activeChartTypography = {color:'warning'}; const ChartSelectionTable = ({tbl_id, chartIds, selectedChartId}) => { @@ -341,7 +353,7 @@ const SelChartOpt = ({chartId, groupKey, header, traceTitles, idx}) => { const {Name} = useBasicOptions({activeTrace: traceNum, groupKey}); return ; }; - const isOpen = !isSpectralOrder(chartId); + const isOpen = !isGroupedChart(chartId); return ( @@ -547,7 +559,7 @@ function applyCascadingAlgo(chartId, chartData, idx, padding) { function canCombine(chartId, selectedChartId) { const activeTrace = 0; // hard-code to only use the first trace from each chart - const {xUnit, yUnit} = getChartData(selectedChartId)?.fireflyData?.[activeTrace]; + const {xUnit, yUnit} = getChartData(selectedChartId)?.fireflyData?.[activeTrace] || {}; if (!xUnit || !yUnit) return false; const chartData = cloneDeep(getChartData(chartId)); diff --git a/src/firefly/js/charts/ui/PlotlyChartArea.jsx b/src/firefly/js/charts/ui/PlotlyChartArea.jsx index f9670b1c19..a04c82e08c 100644 --- a/src/firefly/js/charts/ui/PlotlyChartArea.jsx +++ b/src/firefly/js/charts/ui/PlotlyChartArea.jsx @@ -5,7 +5,7 @@ import {PlotlyWrapper} from './PlotlyWrapper.jsx'; import {showInfoPopup} from '../../ui/PopupUtil.jsx'; import {dispatchChartHighlighted, dispatchChartUpdate, dispatchSetActiveTrace, getAnnotations, getChartData, usePlotlyReact} from '../ChartsCntlr.js'; -import {clearChartConn, flattenAnnotations, handleTableSourceConnections, isSpectralOrder, isScatter2d, +import {clearChartConn, flattenAnnotations, handleTableSourceConnections, isGroupedChart, isScatter2d, makeShapeHoverTrace} from '../ChartUtil.js'; import {useStoreConnector} from 'firefly/ui/SimpleComponent.jsx'; import {Skeleton, useTheme} from '@mui/joy'; @@ -77,7 +77,7 @@ export function PlotlyChartArea({chartId, widthPx, heightPx, thumbnail}) { const {chartWidth, chartHeight} = calculateChartSize(widthPx, heightPx, xyratio, stretch); - const showlegend = data.length > 1; + const showlegend = isGroupedChart(chartId) ? true : get(layout, 'showlegend', data.length > 1); const playout = cloneDeep({showlegend, ...adjustLayout(layout, theme), width: chartWidth, height: chartHeight, annotations, ...hoverLayout}); const style = {float: 'left'}; @@ -250,7 +250,7 @@ function onSelect(chartId) { points = get(evData, 'points', []); points = points.map((o) => [o.pointNumber, curveNumberMap[o.curveNumber]]); let newActiveTrace = activeTrace; - if (!isSpectralOrder(chartId)) { + if (!isGroupedChart(chartId)) { // selected points must belong to the active trace // if no active trace points are selected, // find the trace with the most points in the selection area and make it active diff --git a/src/firefly/js/charts/ui/options/BasicOptions.jsx b/src/firefly/js/charts/ui/options/BasicOptions.jsx index ed94ea05b4..13f54d3c16 100644 --- a/src/firefly/js/charts/ui/options/BasicOptions.jsx +++ b/src/firefly/js/charts/ui/options/BasicOptions.jsx @@ -300,7 +300,7 @@ export function evalChangesFromFields(chartId, tbl_id, fields) { const changes = {showOptions: false}; Object.entries(fields).forEach( ([k,v]) => { if (tbl_id && k.startsWith('_tables.')) { - const [,activeTrace] = /^_tables.data.(\d)/.exec(k) || []; + const [,activeTrace] = /^_tables\.data\.(\d+)\./.exec(k) || []; if (!isUndefined(activeTrace)) { // table id must be set for a data change changes[`data.${activeTrace}.tbl_id`] = data[activeTrace]?.tbl_id || tbl_id; @@ -474,7 +474,7 @@ function filterOptions(options, opts) { */ export const useBasicOptions = ({activeTrace:pActiveTrace, chartId, tbl_id, groupKey, isXNotNumeric, isYNotNumeric, xNoLog, yNoLog, orientation='horizontal'}, deps=[pActiveTrace]) => { - const {activeTrace, data, layout, fireflyLayout, color, ...rest} = getChartProps(chartId, tbl_id, pActiveTrace); + const {activeTrace, data, fireflyData, layout, fireflyLayout, color, ...rest} = getChartProps(chartId, tbl_id, pActiveTrace); xNoLog = xNoLog ?? rest.xNoLog; yNoLog = yNoLog ?? rest.yNoLog; isXNotNumeric = isXNotNumeric ?? rest.isXNotNumeric; @@ -555,10 +555,10 @@ export const useBasicOptions = ({activeTrace:pActiveTrace, chartId, tbl_id, grou options={[ {label: 'height', value: 'fit'}, {label: 'width', value: 'fill'}]} orientation={orientation} {...props}/>), deps), Name: useCallback((props) => (), deps), + orientation={orientation} {...props}/>), [...deps, activeTrace, data?.length, fireflyData]), Color: useCallback((props) => { const colorPicker = (
); - }, deps), + }, [...deps, activeTrace, data?.length]), }; }; diff --git a/src/firefly/js/charts/ui/options/ScatterOptions.jsx b/src/firefly/js/charts/ui/options/ScatterOptions.jsx index 90efaabacb..2d0fb72473 100644 --- a/src/firefly/js/charts/ui/options/ScatterOptions.jsx +++ b/src/firefly/js/charts/ui/options/ScatterOptions.jsx @@ -1,18 +1,21 @@ import React, {useCallback, useEffect} from 'react'; -import {get, isUndefined, omit, range, isString, defaultsDeep, memoize} from 'lodash'; +import {get, has, isUndefined, omit, range, isString, defaultsDeep} from 'lodash'; import {Stack, Typography} from '@mui/joy'; import {Expression} from '../../../util/expr/Expression.js'; -import {getChartData, hasUpperLimits} from '../../ChartsCntlr.js'; -import {getChartProps, getMinScatterGLRows, isSpectralOrder} from '../../ChartUtil.js'; +import {dispatchChartUpdate, getChartData, hasUpperLimits} from '../../ChartsCntlr.js'; +import { + clearChartConn, getChartProps, getGroupableColumns, getGroupByColumn, getMinScatterGLRows, GROUP_BY_NONE, + isGroupByAllowed, isGroupedChart, makeScatterGroupByChanges +} from '../../ChartUtil.js'; import {FieldGroup} from '../../../ui/FieldGroup.jsx'; import {VALUE_CHANGE} from '../../../fieldGroup/FieldGroupCntlr.js'; import {ListBoxInputField} from '../../../ui/ListBoxInputField.jsx'; -import {basicFieldReducer, LayoutOptions, submitChanges, useBasicOptions,} from './BasicOptions.jsx'; +import {basicFieldReducer, evalChangesFromFields, LayoutOptions, submitChanges, useBasicOptions,} from './BasicOptions.jsx'; import {toBoolean, updateSet} from '../../../util/WebUtil.js'; import {useStoreConnector} from '../../../ui/SimpleComponent.jsx'; -import {getColValStats} from '../../TableStatsCntlr.js'; +import {dispatchLoadTblStats, getColValStats} from '../../TableStatsCntlr.js'; import {ColumnOrExpression} from '../ColumnOrExpression.jsx'; import { Error_X, Error_Y, errorFieldKey, errorMinusFieldKey, errorShowFieldKey, errorTypeFieldKey, getDefaultErrorType @@ -26,9 +29,10 @@ import { CollapsibleItem } from '../../../ui/panel/CollapsiblePanel.jsx'; import {hideColSelectPopup} from '../ColSelectView.jsx'; -import {CheckboxGroupInputField} from '../../../ui/CheckboxGroupInputField.jsx'; import {getFieldVal} from '../../../fieldGroup/FieldGroupUtils.js'; +import {dispatchComponentStateChange} from '../../../core/ComponentCntlr.js'; +const TRACE_OPTIONS_COMPONENT_KEY = 'chart-scatter-options'; /** @@ -58,8 +62,8 @@ export function ScatterOptions({activeTrace:pActiveTrace, tbl_id:ptbl_id, chartI groupKey = groupKey || `${chartId}-scatter-${activeTrace}`; const {tbl_id, tablesource, dataType} = getChartProps(chartId, ptbl_id, activeTrace); - const {UseSpectrum, X, Y, Yerrors, Xerrors, Ymax, Ymin} = useScatterInputs({activeTrace, tbl_id, chartId, groupKey}); - const showUseSpectrum = !isSpectralOrder(chartId) && dataType === spectrumType; + const {UseSpectrum, X, Y, Yerrors, Xerrors, Ymax, Ymin, GroupBy} = useScatterInputs({activeTrace, tbl_id, chartId, groupKey}); + const showUseSpectrum = !isGroupedChart(chartId) && dataType === spectrumType; const reducerFunc = fieldReducer({chartId, activeTrace, tbl_id}); reducerFunc.ver = chartId+activeTrace+tbl_id; @@ -85,6 +89,7 @@ export function ScatterOptions({activeTrace:pActiveTrace, tbl_id:ptbl_id, chartI {(yLimitUI() || hasUpperLimits(chartId, activeTrace)) && } + } @@ -101,19 +106,29 @@ export function ScatterCommonOptions({activeTrace:pActiveTrace, tbl_id:ptbl_id, const {activeTrace, tbl_id, noColor, multiTrace} = getChartProps(chartId, ptbl_id, pActiveTrace); const {Symbol, ColorMap, ColorSize, ColorScale, Mode} = useScatterInputs({activeTrace, tbl_id, chartId, groupKey}); - const {Name, Color} = useBasicOptions({activeTrace, tbl_id, chartId, groupKey}); - const colValStats = getColValStats(tbl_id); - const isOrder = isSpectralOrder(chartId); + const {ChooseTrace, Name, Color} = useBasicOptions({activeTrace, tbl_id, chartId, groupKey}); + const colValStats = useTableColValStats(tbl_id); + const isGrouped = isGroupedChart(chartId); + + useEffect(() => { + dispatchComponentStateChange(TRACE_OPTIONS_COMPONENT_KEY, {isOpen: isGrouped}); + }, [chartId, isGrouped]); return ( - + {isGrouped && ( + + You can modify the selected trace: + + + )} {multiTrace && } {!noColor && } - {colValStats && !isOrder && ( + {colValStats && !isGrouped && ( @@ -233,6 +248,20 @@ export function submitChangesScatter({chartId, activeTrace, fields, tbl_id, rend }); Object.assign(changes, fields); + + const groupByKey = groupByFieldKey(activeTrace); + const groupByColumn = fields[groupByKey] || GROUP_BY_NONE; + const currentGroupBy = getGroupByColumn(chartId) || GROUP_BY_NONE; + const groupByChanged = groupByColumn !== currentGroupBy; + if (chartId && has(fields, groupByKey) && groupByChanged && isGroupByAllowed(chartId, activeTrace)) { + const groupedFields = omit(changes, groupByKey); + const evalChanges = evalChangesFromFields(chartId, tbl_id, groupedFields); + const groupChanges = makeScatterGroupByChanges({chartId, tbl_id, activeTrace, groupByColumn, changes: evalChanges}); + clearChartConn({chartId}); + dispatchChartUpdate({chartId, changes: groupChanges}); + dispatchComponentStateChange(TRACE_OPTIONS_COMPONENT_KEY, {isOpen: Boolean(groupByColumn)}); + return; + } submitChanges({chartId, fields: changes, tbl_id, renderTreeId}); } @@ -255,11 +284,12 @@ function getTraceType(chartId, tbl_id, activeTrace) { /* * This function returns a collection of components using `useCallback`, ensuring they are not recreated between re-renders. - * To modify this behavior, you can set the `deps` parameter accordingly. + * To modify this behavior, you can set the `baseDeps` parameter accordingly. */ -export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, chartId, groupKey, orientation='horizontal'}, deps=[]) => { +export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, chartId, groupKey, orientation='horizontal'}, baseDeps=[pActiveTrace]) => { const {activeTrace, tbl_id, data, fireflyData, mappings} = getChartProps(chartId, ptbl_id, pActiveTrace); - const colValStats = getColValStats(tbl_id); + const colValStats = useTableColValStats(tbl_id); + const deps = [...baseDeps, colValStats]; const strOrNull = (v) => isString(v) ? v : undefined; const withDefaults = (props) => { @@ -297,6 +327,20 @@ export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char nullAllowed={false} colValStats={colValStats} groupKey={groupKey} {...withDefaults(props)}/>), deps), + GroupBy: useCallback((props) => { + const groupableCols = chartId && isGroupByAllowed(chartId, activeTrace) ? getGroupableColumns(tbl_id) : []; + if (groupableCols.length === 0) return null; + return ( + ({label: label || name, value: name})) + ]} + {...withDefaults(props)}/>); + }, [chartId, tbl_id, activeTrace, data?.length, ...deps]), Xerrors: useCallback((props) => (), deps), Yerrors: useCallback((props) => (), deps), Xmin: useCallback((props) => ( getColValStats(tbl_id), [tbl_id]); + + useEffect(() => { + if (!tbl_id) return; + const request = getTblById(tbl_id)?.request; + if (request) dispatchLoadTblStats(request); + }, [tbl_id]); + return colValStats; +} + +function groupByFieldKey(activeTrace) { + return `fireflyData.${activeTrace}.groupBy.column`; +} diff --git a/src/firefly/js/charts/ui/options/SpectralLines.jsx b/src/firefly/js/charts/ui/options/SpectralLines.jsx index 6a938728a2..87c57052fa 100644 --- a/src/firefly/js/charts/ui/options/SpectralLines.jsx +++ b/src/firefly/js/charts/ui/options/SpectralLines.jsx @@ -1,10 +1,12 @@ -import React, {useEffect} from 'react'; +import React, {useContext, useEffect} from 'react'; import {get} from 'lodash'; import {Stack} from '@mui/joy'; import {SwitchInputField} from 'firefly/ui/SwitchInputField'; import {CheckboxGroupInputField} from 'firefly/ui/CheckboxGroupInputField'; import {CollapsibleGroup, CollapsibleItem} from 'firefly/ui/panel/CollapsiblePanel'; -import {useStoreConnector, useFieldValueOnly} from 'firefly/ui/SimpleComponent'; +import {useStoreConnector} from 'firefly/ui/SimpleComponent'; +import {FieldGroupCtx} from 'firefly/ui/FieldGroup'; +import {getFieldVal} from 'firefly/fieldGroup/FieldGroupUtils'; import {getChartData} from '../../ChartsCntlr.js'; import {isKnownRefPos} from 'firefly/voAnalyzer/SpectrumDM'; import {canUnitConv, convertUnitValue} from '../../dataTypes/SpectrumUnitConversion.js'; @@ -112,6 +114,7 @@ async function ensureRecommendedLines() { } export function SpectralLinesOptions({activeTrace, chartId}) { + const {groupKey} = useContext(FieldGroupCtx); useEffect(() => { // pre-register tbl_ui_id so columns/columnWidths get populated once loaded (TablePanel mounts later, too late) dispatchTableUiUpdate({tbl_ui_id: RECOMMENDED_LINES_TBL_UI_ID, tbl_id: RECOMMENDED_LINES_TBL_ID}); @@ -126,11 +129,13 @@ export function SpectralLinesOptions({activeTrace, chartId}) { const hasSpectralFrame = useStoreConnector(() => isKnownRefPos(getChartData(chartId)?.fireflyData?.[activeTrace]?.spectralFrame?.refPos), [chartId, activeTrace]); - const isEnabledField = useFieldValueOnly(ENABLED_KEY, false); - const isEnabled = hasSpectralFrame && isEnabledField; + + // read the current trace's field-group value, fall back to persisted state while it mounts + const isEnabledField = useStoreConnector(() => getFieldVal(groupKey, ENABLED_KEY), [groupKey]); + const isEnabled = hasSpectralFrame && (isEnabledField ?? initialEnabled); // TODO: combine different source tables to a client-side table - const sourceOptions = useFieldValueOnly(SOURCE_OPTIONS_KEY, initialSourceOptions); + const sourceOptions = useStoreConnector(() => getFieldVal(groupKey, SOURCE_OPTIONS_KEY), [groupKey]) ?? initialSourceOptions; const activeTblId = sourceOptionToTblId(sourceOptions); const activeTblUiId = activeTblId && `${activeTblId}-ui`; diff --git a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx index 93e1bf9035..90452ca628 100644 --- a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx +++ b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx @@ -21,13 +21,13 @@ import {ListBoxInputField} from '../../../ui/ListBoxInputField.jsx'; import {fieldReducer, submitChangesScatter, ScatterCommonOptions, useScatterInputs} from './ScatterOptions.jsx'; import {VALUE_CHANGE} from '../../../fieldGroup/FieldGroupCntlr.js'; import {updateSet, toBoolean} from '../../../util/WebUtil.js'; -import {isSpectralOrder, getChartProps} from '../../ChartUtil.js'; +import {getChartProps, isGroupedChart} from '../../ChartUtil.js'; import {LayoutOptions, useBasicOptions} from './BasicOptions.jsx'; import {getSpectrumProps} from '../../dataTypes/FireflySpectrum.js'; import {getFieldVal, revalidateFields} from 'firefly/fieldGroup/FieldGroupUtils'; import {isFloat} from 'firefly/util/Validate'; +import {quoteNonAlphanumeric} from 'firefly/util/expr/Variable'; import {ValidationField} from 'firefly/ui/ValidationField'; -import {sprintf} from 'firefly/externalSource/sprintf'; import {RadioGroupInputField} from 'firefly/ui/RadioGroupInputField'; import {Box, FormLabel, Stack, Typography} from '@mui/joy'; import {CollapsibleGroup} from 'firefly/ui/panel/CollapsiblePanel'; @@ -45,7 +45,7 @@ export function SpectrumOptions ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char const {xErrArray, yErrArray, xMax, xMin, yMax, yMin, xUnit, yUnit} = getSpectrumProps(tbl_id); const {Xunit, Yunit, SpectralFrame, SpectralLines} = useSpectrumInputs({activeTrace, tbl_id, chartId, groupKey}); - const {UseSpectrum, X, Xmax, Xmin, Y, Ymax, Ymin, Yerrors, Xerrors} = useScatterInputs({activeTrace, tbl_id, chartId, groupKey}); + const {UseSpectrum, X, Xmax, Xmin, Y, Ymax, Ymin, Yerrors, Xerrors, GroupBy} = useScatterInputs({activeTrace, tbl_id, chartId, groupKey}); const {XaxisTitle, YaxisTitle} = useBasicOptions({activeTrace, tbl_id, chartId, groupKey}); const reducerFunc = spectrumReducer({chartId, activeTrace, tbl_id}); @@ -75,7 +75,7 @@ export function SpectrumOptions ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char '.ff-ColumnFld': inputFullWidthSx, '.ff-Input .MuiInput-root': inputFullWidthSx, }}> - {!isSpectralOrder(chartId) && } + {!isGroupedChart(chartId) && } {xErrArray && } @@ -92,6 +92,7 @@ export function SpectrumOptions ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char {yMin && } + @@ -145,13 +146,14 @@ export function spectrumReducer({chartId, activeTrace, tbl_id}) { /* ------------------------------ Redshift correction handling ------------------------------ */ const getRedshiftCorrectedExpr = ({cname, spectralFrame, sfOption, redshift=undefined}) => { - const {refPos, redshift: customRedshift} = spectralFrame; - const multiplyBy = refPos.toUpperCase() === REF_POS.CUSTOM ? ` * (1 + ${customRedshift ?? '0'})` : ''; + const {refPos, redshift: customRedshift} = spectralFrame || {}; + const normalizedRefPos = refPos?.toUpperCase?.() || REF_POS.TOPOCENTER; + const multiplyBy = normalizedRefPos === REF_POS.CUSTOM ? ` * (1 + ${customRedshift ?? '0'})` : ''; const divideBy = sfOption === 'rest' && redshift ? ` / (1 + ${redshift})` : ''; - let expr = `"%s"${multiplyBy}${divideBy}`; + let expr = `${quoteNonAlphanumeric(cname)}${multiplyBy}${divideBy}`; // multiplyBy = divideBy when correcting a spectrum with custom redshift to the rest frame - if (sfOption === 'rest' && customRedshift === redshift) expr = '"%s"'; - return sprintf(expr, cname); + if (sfOption === 'rest' && customRedshift === redshift) expr = quoteNonAlphanumeric(cname); + return expr; }; const getCombinedExpr = (cname, redshiftCorrParams, unitConvParams) => { @@ -257,14 +259,41 @@ export const applyUnitConversion = ({fireflyData, data, inFields, axisType, newU export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, renderTreeId}) { const {data, fireflyData} = getChartData(chartId); const {spectralAxis={}, fluxAxis={}} = getSpectrumDM(getTblById(tbl_id)) || {}; + const chartProps = getChartProps(chartId, tbl_id, activeTrace); + + //The table source can be briefly unavailable while grouped traces are being rebuilt. Keep all + //canonical table mappings in the submit payload so a trace options change does not accidntally clear + //the spectral axes or their error/limit columns + Object.entries(chartProps.mappings || {}).forEach(([key, value]) => { + const fieldKey = key.startsWith('fireflyData.') + ? `_tables.${key}` + : `_tables.data.${activeTrace}.${key}`; + if (!fields[fieldKey] && value) fields = updateSet(fields, [fieldKey], value); + }); + if (!fields[`_tables.data.${activeTrace}.x`] && spectralAxis.value) { + fields = updateSet(fields, [`_tables.data.${activeTrace}.x`], spectralAxis.value); + } + if (!fields[`_tables.data.${activeTrace}.y`] && fluxAxis.value) { + fields = updateSet(fields, [`_tables.data.${activeTrace}.y`], fluxAxis.value); + } // get units and spectral frame options from the fields of active trace const xUnit = fields[`fireflyData.${activeTrace}.xUnit`]; const yUnit = fields[`fireflyData.${activeTrace}.yUnit`]; // undefined if no field for yUnit - const sfOptionFields = Object.fromEntries(Object.entries(SFOptionFieldKeys(activeTrace)) - .map(([subKey, fieldKey])=>[subKey, fields[fieldKey]])); + const currentSFOptionFields = getEffectiveSFOptionFields(fireflyData?.[activeTrace]); + const sfFieldKeys = SFOptionFieldKeys(activeTrace); + const sfOptionFields = { + value: fields[sfFieldKeys.value] ?? currentSFOptionFields.value, + redshift: fields[sfFieldKeys.redshift] ?? currentSFOptionFields.redshift, + userSpecified: fields[sfFieldKeys.userSpecified] ?? currentSFOptionFields.userSpecified + }; + const xUnitChanged = xUnit !== undefined && xUnit !== fireflyData?.[activeTrace]?.xUnit; + const yUnitChanged = yUnit !== undefined && yUnit !== fireflyData?.[activeTrace]?.yUnit; + const spectralFrameChanged = !isEqual(sfOptionFields, currentSFOptionFields); - // when units or spectral frame options change, apply it to the other/inactive traces as well + //Only propagate axis settings when an axis setting actually changed. In particular, changing + //a grouped trace's style or symbol must not cause missing spectralFrameOption metadata on a + //sibling trace to trigger a unit conversion and rewrite lambda/Flux expressions. range(data.length).forEach((idx) => { if (idx !== activeTrace) { // get units and spectral frame options from the chart data of inactive trace @@ -274,7 +303,8 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren .map((key)=>[key, fireflyData?.[idx]?.spectralFrameOption?.[key]])); // set the fields of inactive trace same as that of active trace, if they don't match - if (!isEqual(sfOptionTrace, sfOptionFields) || (xUnitTrace!==xUnit && canUnitConv({from: xUnitTrace, to: xUnit}))) { + if ((xUnitChanged || spectralFrameChanged) && + (!isEqual(sfOptionTrace, sfOptionFields) || (xUnitTrace!==xUnit && canUnitConv({from: xUnitTrace, to: xUnit})))) { fields = updateSet(fields, [`fireflyData.${idx}.xUnit`], xUnit); Object.entries(SFOptionFieldKeys(idx)).forEach(([subKey, fieldKey])=>{ fields = updateSet(fields, [fieldKey], sfOptionFields[subKey]); @@ -283,15 +313,15 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren // applying unit conversion on X axis, will also apply redshift correction so no need to call it separately fields = applyUnitConversion({fireflyData, data, inFields:fields, axisType:'x', newUnit:xUnit, traceNum:idx, axis:spectralAxis}); } - if (canUnitConv({from: yUnitTrace, to: yUnit})) { + if (yUnitChanged && canUnitConv({from: yUnitTrace, to: yUnit})) { fields = updateSet(fields, [`fireflyData.${idx}.yUnit`], yUnit || yUnitTrace); fields = applyUnitConversion({fireflyData, data, inFields:fields, axisType:'y', newUnit:yUnit || yUnitTrace, traceNum:idx, axis:fluxAxis}); } } }); - // when show/hide error changes for spectrum with 'order', apply it to other traces as well - if (isSpectralOrder(chartId)) { + // when show/hide error changes for grouped spectrum traces, apply it to other traces as well + if (isGroupedChart(chartId)) { const xShowError = fields[errorShowFieldKey(activeTrace, 'x')] || 'false'; const yShowError = fields[errorShowFieldKey(activeTrace, 'y')] || 'false'; range(data.length).forEach((idx) => { @@ -310,9 +340,14 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren }); } - // handle spectral lines ----- - const spectralLinesEnabled = toBoolean(fields['spectralLines.enabled']); - const spectralLinesTblId = sourceOptionToTblId(fields['spectralLines.sourceOptions']); + //preserve chart state while spectral-line fields might be temporarily unmounted while switching traces + const currentSpectralLines = getChartData(chartId)?.fireflyLayout?.spectralLines ?? {}; + const spectralLinesEnabled = fields['spectralLines.enabled'] === undefined + ? toBoolean(currentSpectralLines.enabled) + : toBoolean(fields['spectralLines.enabled']); + const spectralLinesTblId = fields['spectralLines.sourceOptions'] === undefined + ? currentSpectralLines.source + : sourceOptionToTblId(fields['spectralLines.sourceOptions']); fields = omit(fields, ['spectralLines.enabled', 'spectralLines.sourceOptions']); // persisted only so UI controls can seed their initial state from the chart data next time this dialog opens @@ -337,6 +372,17 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren submitChangesScatter({chartId, activeTrace, fields, tbl_id, renderTreeId}); } +function getEffectiveSFOptionFields(trace={}) { + const spectralFrame = trace.spectralFrame || {}; + const spectralFrameOption = trace.spectralFrameOption || {}; + const refPos = spectralFrame.refPos?.toUpperCase?.(); + return { + value: spectralFrameOption.value ?? (refPos === REF_POS.TOPOCENTER ? 'observed' : 'rest'), + redshift: spectralFrameOption.redshift ?? 'userSpecified', + userSpecified: spectralFrameOption.userSpecified ?? '0' + }; +} + function Units({activeTrace, value, axis, ...rest}) { @@ -365,24 +411,24 @@ function ReadOnlyField({value, label, ...props}) { /* * This function returns a collection of components using `useCallback`, ensuring they are not recreated between re-renders. - * To modify this behavior, you can set the `deps` parameter accordingly. */ -export const useSpectrumInputs = ({chartId, groupKey}, deps=[]) => { +export const useSpectrumInputs = ({activeTrace:pActiveTrace, chartId, groupKey}) => { - const {activeTrace=0, fireflyData={}} = getChartData(chartId); + const {activeTrace:chartActiveTrace=0, fireflyData={}} = getChartData(chartId); + const activeTrace = pActiveTrace ?? chartActiveTrace; return { - Xunit: useCallback((props={}) => , deps), - Yunit: useCallback((props={}) => , deps), + Xunit: useCallback((props={}) => , [activeTrace, fireflyData]), + Yunit: useCallback((props={}) => , [activeTrace, fireflyData]), SpectralFrame: useCallback((props={}) => { const allProps = {label: 'Spectral frame:', ...props}; const sfRefPos = fireflyData[activeTrace].spectralFrame.refPos.toUpperCase(); return isKnownRefPos(sfRefPos) //only show options when TOPOCENTER or CUSTOM ? : ; - }, deps), + }, [activeTrace, fireflyData, groupKey]), SpectralLines: useCallback((props={}) => - , deps), + , [activeTrace, chartId]), }; }; From 6389c93612f45a8a111105ef8178306806572a17 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Thu, 6 Aug 2026 21:09:13 -0700 Subject: [PATCH 02/11] fix several regression and new bugs --- src/firefly/js/charts/ChartUtil.js | 32 ++++++++-- src/firefly/js/charts/ChartsCntlr.js | 60 ++++++++++++------- .../js/charts/dataTypes/FireflyGenericData.js | 26 +++++--- .../js/charts/dataTypes/FireflySpectrum.js | 16 +++-- 4 files changed, 93 insertions(+), 41 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 5364223828..6fb39f7083 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -16,10 +16,10 @@ import shallowequal from 'shallowequal'; import {getAppOptions} from '../core/AppDataCntlr.js'; import { - COL_TYPE, ensureEnumVals, getColumn, getColumns, getColumnType, getMetaEntry, getTblById, isColumnType, + COL_TYPE, ensureEnumVals, getColumn, getColumnIdx, getColumns, getColumnType, getMetaEntry, getTblById, isColumnType, isFullyLoaded, isTableLoaded, splitVals, stripColumnNameQuotes, SYS_COLUMNS, watchTableChanges } from '../tables/TableUtil.js'; -import {TABLE_HIGHLIGHT, TABLE_LOADED, TABLE_SELECT, TABLE_SORT} from '../tables/TablesCntlr.js'; +import {TABLE_FILTER, TABLE_HIGHLIGHT, TABLE_LOADED, TABLE_SELECT, TABLE_SORT} from '../tables/TablesCntlr.js'; import {getSpectrumDM} from '../voAnalyzer/SpectrumDM.js'; import {findTableCenterColumns} from '../voAnalyzer/TableAnalysis.js'; import {dispatchLoadTblStats, getColValStats} from './TableStatsCntlr.js'; @@ -87,6 +87,11 @@ export function getTableSourceMappings(flattenData={}) { }, {}); } +export function getOriginalRowIndexes(tableModel) { + const origIdx = getColumnIdx(tableModel, 'ORIG_IDX'); + return origIdx >= 0 ? tableModel?.tableData?.data?.map((row) => row[origIdx]) : undefined; +} + /** * Does the application only support single trace * @returns {*} true, if all charts are single trace @@ -773,7 +778,10 @@ export function handleTableSourceConnections({chartId, data, fireflyData}) { const tablesources = makeTableSources(chartId, data, fireflyData); const {tablesources:oldTablesources=[], activeTrace, curveNumberMap=[]} = getChartData(chartId); - const hasTablesources = Array.isArray(tablesources) && tablesources.find((ts) => !isEmpty(ts)); + // Partial data updates contain arrays, not table-source metadata. Keep the + // existing watchers for those updates; only reconnect on source changes. + const hasTablesources = Array.isArray(tablesources) && tablesources.find((ts) => + ts?.tbl_id && !isEmpty(ts.mappings)); if (!hasTablesources) return; const numTraces = Math.max(tablesources.length, oldTablesources.length); @@ -898,7 +906,15 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // may not need to call it as often. if this becomes a performance issue, then optimize. dispatchLoadTblStats(tableModel.request); + // Mark the trace loading while its type-specific fetch obtains the + // current table data (and, for grouped traces, its group subset). const changes = getDataChangesForMappings({mappings, traceNum}); + if (action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER) { + changes['layout.xaxis.autorange'] = true; + changes['layout.xaxis.range'] = undefined; + changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.range'] = undefined; + } // save original table file path const resultSetIDNow = get(tableModel, 'tableMeta.resultSetID'); @@ -911,7 +927,10 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // fetch data for both Firefly recognized or unrecognized plotly chart types if (tablesource.fetchData) { - tablesource.fetchData(chartId, traceNum, tablesource); + const fetchSource = action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER + ? {...tablesource, resetAxes: true} + : tablesource; + tablesource.fetchData(chartId, traceNum, fetchSource); } } } @@ -1496,9 +1515,10 @@ export function getChartProps(chartId, tbl_id, activeTrace) { export function getTblIdFromChart(chartId, traceNum) { - const {data, fireflyData, activeTrace} = getChartData(chartId) || {}; + const {data, fireflyData, tablesources, activeTrace} = getChartData(chartId) || {}; traceNum = traceNum ?? activeTrace; - return data?.[traceNum]?.tbl_id || fireflyData?.[traceNum]?.tbl_id; + return tablesources?.[traceNum]?.tbl_id || data?.[traceNum]?.tbl_id || fireflyData?.[traceNum]?.tbl_id || + fireflyData?.[traceNum]?.groupBy?.baseTable?.tbl_id || fireflyData?.[traceNum]?.baseTable?.tbl_id; } export function hasTracesFromSameTable(chartId) { diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index 7cf56b329f..fd05fbf61f 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -16,13 +16,13 @@ import {dispatchAddViewerItems, dispatchUpdateCustom, dispatchRemoveViewerItems, import {DEFAULT_PLOT2D_VIEWER_ID} from '../visualize/VisConst'; import {applyDefaults, flattenAnnotations, formatColExpr, getPointIdx, getRowIdx, handleTableSourceConnections, clearChartConn, newTraceFrom, setupTableWatcher, HIGHLIGHTED_PROPS, SELECTED_PROPS, TBL_SRC_PATTERN, - getSelIndexes, isGroupedChart, combineAllTraceFrom} from './ChartUtil.js'; + getSelIndexes, getTblIdFromChart, isGroupedChart, combineAllTraceFrom} from './ChartUtil.js'; import {FilterInfo} from '../tables/FilterInfo.js'; import {SelectInfo} from '../tables/SelectInfo.js'; import {REINIT_APP} from '../core/CoreConst'; import {getAppOptions} from '../core/AppDataCntlr.js'; import {showInfoPopup} from '../ui/PopupUtil.jsx'; -import {makeHistogramParams, makeXYPlotParams, getDefaultChartProps} from './ChartUtil.js'; +import {makeHistogramParams, makeXYPlotParams, getDefaultChartProps, getChartProps} from './ChartUtil.js'; import {adjustColorbars, hasFireflyColorbar} from './dataTypes/FireflyHeatmap.js'; export const CHART_SPACE_PATH = 'charts'; @@ -339,7 +339,7 @@ function chartUpdate(action) { // lazy table connection const {mounted} = getChartData(chartId); - if (mounted > 0) { + if (mounted > 0 && (!isEmpty(data) || !isEmpty(fireflyData))) { handleTableSourceConnections({chartId, data, fireflyData}); } }; @@ -349,7 +349,7 @@ function chartHighlight(action) { return (dispatch) => { const {chartId, highlighted=0, chartTrigger=false} = action.payload; // TODO: activeTrace is not implemented. switch to trace.. then highlight(?) - const {data, fireflyData, tablesources, activeTrace:activeDataTrace=0, selected} = getChartData(chartId); + const {data, fireflyData, activeTrace:activeDataTrace=0, selected} = getChartData(chartId); const {traceNum=activeDataTrace, traceName} = action.payload; // highlighted trace can be selected or highlighted trace of the data trace @@ -359,16 +359,21 @@ function chartHighlight(action) { const ttype = get(data, [traceNum, 'type'], 'scatter'); - if (!isEmpty(tablesources) && ttype.includes('scatter')) { + if (ttype.includes('scatter')) { // activeTrace is different from activeDataTrace if a selected point highlighted, for example - const {tbl_id} = tablesources[activeDataTrace] || {}; - if (!tbl_id) return; + const tbl_id = getTblIdFromChart(chartId, activeDataTrace); + if (!tbl_id || !getTblById(tbl_id)) return; // avoid updating chart twice // update only as a response to table highlight change if (!chartTrigger) { const traceAnnotations = get(fireflyData, `${traceNum}.annotations`); const hlTrace = newTraceFrom(data[traceNum], [highlighted], HIGHLIGHTED_PROPS, traceAnnotations); dispatchChartUpdate({chartId, changes: {highlighted: hlTrace}}); + } else if (data[traceNum]) { + // Update the visual highlight immediately; the table watcher may be rebuilding. + const traceAnnotations = get(fireflyData, `${traceNum}.annotations`); + const hlTrace = newTraceFrom(data[traceNum], [highlighted], HIGHLIGHTED_PROPS, traceAnnotations); + dispatchChartUpdate({chartId, changes: {highlighted: hlTrace}}); } let traceData = data[traceNum]; @@ -392,7 +397,7 @@ function chartHighlight(action) { function chartSelect(action) { return (dispatch) => { const {chartId, selIndexes=[], chartTrigger=false} = action.payload; - const {activeTrace=0, data, fireflyData, tablesources} = getChartData(chartId); + const {activeTrace=0, data, fireflyData} = getChartData(chartId); // when skipping hover, selecting chart points does not work // disable chart select in this case @@ -401,13 +406,16 @@ function chartSelect(action) { // avoid updating chart twice // don't update before table select if (chartTrigger) { - if (!isEmpty(tablesources)) { - const {tbl_id} = tablesources[activeTrace] || {}; - const {totalRows} = getTblById(tbl_id); + const tbl_id = getTblIdFromChart(chartId, activeTrace); + const tableModel = tbl_id && getTblById(tbl_id); + if (tableModel) { + const {totalRows} = tableModel; const selectInfoCls = SelectInfo.newInstance({rowCount: totalRows}); selIndexes.forEach(([ptIdx, traceIdx]) => selectInfoCls.setRowSelect(getRowIdx(data[traceIdx], ptIdx), true)); TablesCntlr.dispatchTableSelect(tbl_id, selectInfoCls.data); + // Keep the chart overlay in sync even while table-source watchers rebuild. + dispatchChartSelect({chartId, selIndexes}); } } else { let selected = undefined; @@ -427,10 +435,12 @@ function chartSelect(action) { function chartFilterSelection(action) { return (dispatch) => { const {chartId} = action.payload; - const {activeTrace=0, selection, tablesources} = getChartData(chartId); - if (!isEmpty(tablesources)) { - const {tbl_id, mappings} = tablesources[activeTrace]; - const numericCols = getColumns(getTblById(tbl_id), COL_TYPE.NUMBER).map((c) => c.name); + const {activeTrace=0, selection} = getChartData(chartId) || {}; + const tbl_id = getTblIdFromChart(chartId, activeTrace); + const tableModel = tbl_id && getTblById(tbl_id); + const {mappings={}} = tableModel ? getChartProps(chartId, tbl_id, activeTrace) : {}; + if (tableModel && selection && mappings.x && mappings.y) { + const numericCols = getColumns(tableModel, COL_TYPE.NUMBER).map((c) => c.name); let {x,y} = mappings; let upperLimit = get(mappings, `fireflyData.${activeTrace}.yMax`); @@ -451,7 +461,7 @@ function chartFilterSelection(action) { } const [xMin, xMax] = get(selection, 'range.x', []); const [yMin, yMax] = get(selection, 'range.y', []); - const {request} = getTblById(tbl_id); + const {request} = tableModel; const filterInfoCls = FilterInfo.parse(request.filters); filterInfoCls.setFilter(x, '> ' + xMin); @@ -462,12 +472,18 @@ function chartFilterSelection(action) { // filters are processed by db, column expressions need to use syntax db understands // filters can be set for any column type, numeric or non-numeric - const allColumns = getColumns(getTblById(tbl_id)).map((c) => c.name); + const allColumns = getColumns(tableModel).map((c) => c.name); const formatKey = (k) => formatColExpr({colOrExpr:k, quoted:true, colNames:allColumns}); const newRequest = Object.assign({}, request, {filters: filterInfoCls.serialize(formatKey)}); TablesCntlr.dispatchTableFilter(newRequest); - dispatchChartUpdate({chartId, changes:{selection: undefined}}); + dispatchChartUpdate({chartId, changes:{ + selection: undefined, + 'layout.xaxis.autorange': true, + 'layout.xaxis.range': undefined, + 'layout.yaxis.autorange': true, + 'layout.yaxis.range': undefined + }}); } }; } @@ -475,14 +491,14 @@ function chartFilterSelection(action) { function setActiveTrace(action) { return (dispatch) => { const {chartId, activeTrace} = action.payload; - const {data, fireflyData, tablesources, curveNumberMap} = getChartData(chartId); - const changes = getActiveTraceChanges({chartId, activeTrace, data, fireflyData, tablesources, curveNumberMap}); + const {data, fireflyData, curveNumberMap} = getChartData(chartId); + const changes = getActiveTraceChanges({chartId, activeTrace, data, fireflyData, curveNumberMap}); dispatchChartUpdate({chartId, changes}); }; } -function getActiveTraceChanges({chartId, activeTrace, data, fireflyData, tablesources, curveNumberMap}) { - const tbl_id = get(tablesources, [activeTrace, 'tbl_id']); +function getActiveTraceChanges({chartId, activeTrace, data, fireflyData, curveNumberMap}) { + const tbl_id = getTblIdFromChart(chartId, activeTrace); let selected = undefined; let highlighted = undefined; let curveMap = undefined; diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index 77934593f4..84ca45bbb3 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -2,10 +2,11 @@ * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ import {get, isArray, isUndefined, uniqueId, isNil} from 'lodash'; -import {getTblById, getColumns, getColumn, getColumnIdx, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; +import {getTblById, getColumns, getColumn, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; import {cloneRequest, makeSubQueryRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; +import {getFiltersAsSql} from '../../tables/FilterInfo.js'; import {dispatchChartUpdate, dispatchError, getChartData, getTraceSymbol, hasUpperLimits, hasLowerLimits} from '../ChartsCntlr.js'; -import {formatColExpr, getDataChangesForMappings, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; +import {formatColExpr, getDataChangesForMappings, getOriginalRowIndexes, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; import {getTraceTSEntries as heatmapTSGetter} from './FireflyHeatmap.js'; import {errorTypeFieldKey} from '../ui/options/Errors.jsx'; @@ -47,7 +48,7 @@ export function getTraceTSEntries({traceTS, chartId, traceNum}) { async function fetchData(chartId, traceNum, tablesource) { - const {tbl_id, mappings} = tablesource; + const {tbl_id, mappings, resetAxes} = tablesource; if (!mappings) { return; } @@ -87,11 +88,16 @@ async function fetchData(chartId, traceNum, tablesource) { // extra changes based on trace type addOtherChanges({changes, chartId, traceNum, tablesource, tableModel: originalTableModel}); - const origIdx = getColumnIdx(tableModel, 'ORIG_IDX'); - if (origIdx >= 0) { - changes[`data.${traceNum}.firefly.rowIdx`] = tableModel.tableData.data.map((row) => row[origIdx]); + if (resetAxes) { + changes['layout.xaxis.autorange'] = true; + changes['layout.xaxis.range'] = undefined; + changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.range'] = undefined; } + const rowIdx = getOriginalRowIndexes(tableModel); + if (rowIdx) changes[`data.${traceNum}.firefly.rowIdx`] = rowIdx; + dispatchChartUpdate({chartId, changes}); const {activeTrace} = getChartData(chartId); if (isUndefined(activeTrace) || activeTrace === traceNum) { @@ -125,9 +131,13 @@ export function createChartTblRequest(chartId, traceNum, tablesource) { }).filter((c, i, a) => a.indexOf(c) === i).// remove duplicates join(', ') // allows to use the same columns, ex. "w1" as "x", "w1" as "marker.color" }, true); - if (fireflyData?.[traceNum]?.filters) { + const groupFilter = fireflyData?.[traceNum]?.filters; + if (groupFilter) { const inclCols = (sreq.inclCols ? sreq.inclCols + ',' : '') + '"ROW_NUM" as "ORIG_IDX"'; - sreq = makeSubQueryRequest(request, sreq.title, sreq.params,{filters: fireflyData?.[traceNum].filters, inclCols, pageSize: MAX_ROW}); + const tableFilter = getFiltersAsSql(tbl_id); + // Keep selection/table filters when adding the generated trace's group predicate. + const filters = tableFilter ? `(${tableFilter}) AND (${groupFilter})` : groupFilter; + sreq = makeSubQueryRequest(request, sreq.title, sreq.params,{filters, inclCols, pageSize: MAX_ROW}); } return sreq; } diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index bffcbeb201..e24d0ea13b 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -6,7 +6,7 @@ import {isEmpty, pickBy, cloneDeep, set, uniqueId} from 'lodash'; import {getTblById, getColumn, doFetchTable, getColumnIdx} from '../../tables/TableUtil.js'; import {getSpectrumDM, REF_POS} from '../../voAnalyzer/SpectrumDM.js'; import {dispatchChartUpdate, dispatchError} from '../ChartsCntlr.js'; -import {getDataChangesForMappings, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; +import {getDataChangesForMappings, getOriginalRowIndexes, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; import {addOtherChanges, createChartTblRequest, getTraceTSEntries as genericTSGetter} from './FireflyGenericData.js'; import {quoteNonAlphanumeric} from '../../util/expr/Variable.js'; @@ -36,7 +36,7 @@ export function getTraceTSEntries({traceTS, chartId, traceNum}) { async function fetchData(chartId, traceNum, tablesource) { - const {tbl_id, mappings} = tablesource; + const {tbl_id, mappings, resetAxes} = tablesource; if (!mappings) { return; } @@ -61,10 +61,16 @@ async function fetchData(chartId, traceNum, tablesource) { // extra changes based on trace type addOtherChanges({changes, chartId, traceNum, tablesource, tableModel: originalTableModel}); + if (resetAxes) { + changes['layout.xaxis.autorange'] = true; + changes['layout.xaxis.range'] = undefined; + changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.range'] = undefined; + } + // add row_idx to pointIdx mappings - const origIdx = getColumnIdx(tableModel, 'ORIG_IDX'); - const rowIdx = tableModel.tableData.data.map((row) => row[origIdx]); - set(changes, [`data.${traceNum}.firefly.rowIdx`], rowIdx); + const rowIdx = getOriginalRowIndexes(tableModel); + if (rowIdx) set(changes, [`data.${traceNum}.firefly.rowIdx`], rowIdx); dispatchChartUpdate({chartId, changes}); updateHighlighted(chartId, traceNum, highlightedRow); From a3835f7a52db35828497a1176bd951ee0cfca065 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Fri, 7 Aug 2026 16:32:42 -0700 Subject: [PATCH 03/11] try new cleaner refactor approach --- src/firefly/js/charts/ChartUtil.js | 193 +++++------------- src/firefly/js/charts/ChartsCntlr.js | 9 +- src/firefly/js/charts/ui/PlotlyChartArea.jsx | 2 +- .../js/charts/ui/options/ScatterOptions.jsx | 5 +- .../js/charts/ui/options/SpectrumOptions.jsx | 33 +-- 5 files changed, 81 insertions(+), 161 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 6fb39f7083..744a2447c6 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -379,14 +379,15 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group const data = chartData.data || []; const fireflyData = chartData.fireflyData || []; - const tablesources = chartData.tablesources || []; const baseData = simpleCloneDeep(data[activeTrace] || {}); const baseFirefly = simpleCloneDeep(fireflyData[activeTrace] || {}); - const baseTable = getCanonicalTableInfo({ - data, fireflyData, tablesources, original: chartData._original, activeTrace, tbl_id + restoreTableSourceMappings({ + trace: baseData, + fireflyTrace: baseFirefly, + tablesource: chartData.tablesources?.[activeTrace], + traceNum: activeTrace, + tbl_id }); - - applyCanonicalTableMappings(baseData, baseFirefly, baseTable); restoreGroupByBaseStyle(baseData, baseFirefly); const commonChanges = { @@ -394,8 +395,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group highlighted: undefined, selected: undefined, selection: undefined, - hasSelected: false, - tablesources: [] + hasSelected: false }; if (!groupByColumn) { @@ -405,7 +405,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group data: [baseData], fireflyData: [baseFirefly], activeTrace: 0, - curveNumberMap: [0], + curveNumberMap: makeCurveNumberMap(1, 0), 'layout.showlegend': undefined, 'layout.legend.title.text': undefined }; @@ -422,7 +422,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group data: [baseData], fireflyData: [baseFirefly], activeTrace: 0, - curveNumberMap: [0], + curveNumberMap: makeCurveNumberMap(1, 0), 'layout.showlegend': undefined, 'layout.legend.title.text': undefined }; @@ -435,11 +435,6 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group const fd = simpleCloneDeep(baseFirefly); const color = toRGBA(TRACE_COLORS[idx % TRACE_COLORS.length]); - // Keep every generated trace independently connected to the original table. - // The table-source watcher can then rebuild any trace without depending on - // whichever generated trace happens to be active. - applyCanonicalTableMappings(trace, fd, baseTable); - set(trace, 'name', value); set(trace, 'showlegend', true); set(trace, 'marker.color', color); @@ -451,8 +446,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group fd.groupBy = { column: groupCol.name, value, - baseTraceStyle: getGroupByBaseStyle(baseData), - baseTable + baseTraceStyle: getGroupByBaseStyle(baseData) }; fd.filters = makeGroupFilter(groupCol.name, value); @@ -460,17 +454,22 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group groupFireflyData.push(fd); }); + const newActiveTrace = Math.min(activeTrace, groupData.length - 1); return { ...commonChanges, data: groupData, fireflyData: groupFireflyData, - activeTrace: Math.min(activeTrace, groupData.length - 1), - curveNumberMap: range(groupData.length), + activeTrace: newActiveTrace, + curveNumberMap: makeCurveNumberMap(groupData.length, newActiveTrace), 'layout.showlegend': true, 'layout.legend.title.text': groupCol.label || groupCol.name }; } +function makeCurveNumberMap(traceCount, activeTrace) { + return [...range(traceCount).filter((idx) => idx !== activeTrace), activeTrace]; +} + function makeGroupFilter(columnName, value) { const escapedColumn = columnName.replace(/"/g, '""'); const escapedValue = String(value).replace(/'/g, "''"); @@ -479,11 +478,6 @@ function makeGroupFilter(columnName, value) { function clearGroupBy(trace, fireflyTrace) { restoreGroupByBaseStyle(trace, fireflyTrace); - if (fireflyTrace?.groupBy?.baseTable) { - // Keep the canonical source available after collapsing generated groups. - // The next regroup must not depend on a transient tablesource watcher. - fireflyTrace.baseTable = fireflyTrace.groupBy.baseTable; - } if (fireflyTrace?.groupBy) { delete trace.name; } @@ -507,80 +501,19 @@ function restoreGroupByBaseStyle(trace, fireflyTrace) { Object.entries(flattenObject(baseTraceStyle)).forEach(([path, value]) => set(trace, path, value)); } -function normalizeTableMappings(mappings={}, traceNum) { - const normalized = {data: {}, fireflyData: {}}; - Object.entries(mappings).forEach(([key, value]) => { - if (key.startsWith(`fireflyData.${traceNum}.`)) { - normalized.fireflyData[key.replace(`fireflyData.${traceNum}.`, '')] = value; - } else { - normalized.data[key] = value; - } - }); - return normalized; -} - -function extractTableMappings(trace={}, fireflyTrace={}, traceNum) { - const mappings = {data: {}, fireflyData: {}}; - const flattenData = assign( - flattenObject(trace), - flattenObject(fireflyTrace, `fireflyData.${traceNum}`) - ); - Object.entries(getTableSourceMappings(flattenData)).forEach(([key, mapping]) => { - if (key.startsWith(`fireflyData.${traceNum}.`)) { - mappings.fireflyData[key.replace(`fireflyData.${traceNum}.`, '')] = mapping; +function restoreTableSourceMappings({trace={}, fireflyTrace={}, tablesource={}, traceNum=0, tbl_id}) { + if (tablesource.tbl_id && !trace.tbl_id) trace.tbl_id = tablesource.tbl_id; + if (!trace.tbl_id && tbl_id) trace.tbl_id = tbl_id; + + Object.entries(tablesource.mappings || {}).forEach(([key, value]) => { + if (value === undefined || value === '') return; + const fireflyPrefix = `fireflyData.${traceNum}.`; + if (key.startsWith(fireflyPrefix)) { + set(fireflyTrace, key.slice(fireflyPrefix.length), `tables::${value}`); } else { - mappings.data[key] = mapping; + set(trace, key, `tables::${value}`); } }); - return mappings; -} - -function mergeTableMappings(base={}, current={}) { - return { - data: {...(base.data || {}), ...(current.data || {})}, - fireflyData: {...(base.fireflyData || {}), ...(current.fireflyData || {})} - }; -} - -function getCanonicalTableInfo({data=[], fireflyData=[], tablesources=[], original={}, activeTrace=0, tbl_id}) { - const groupedBase = fireflyData.find((fd) => fd?.groupBy?.baseTable)?.groupBy?.baseTable || - fireflyData.find((fd) => fd?.baseTable)?.baseTable; - const activeSource = tablesources[activeTrace]; - const source = activeSource?.mappings ? activeSource : undefined; - const sourceMappings = normalizeTableMappings(source?.mappings, activeTrace); - const currentMappings = extractTableMappings(data[activeTrace], fireflyData[activeTrace], activeTrace); - const originalTraceNum = original?.data?.[activeTrace] ? activeTrace : 0; - const originalMappings = extractTableMappings( - original?.data?.[originalTraceNum], original?.fireflyData?.[originalTraceNum], activeTrace - ); - const base = groupedBase || {}; - - //Merge fallback mappings first and current trace/source mappings last - //later entries override earlier values while table sources rebuild - const mappingSources = [ - originalMappings, - base.mappings, - sourceMappings, - currentMappings - ]; - const mappings = mappingSources.reduce( - (merged, source) => mergeTableMappings(merged, source), {} - ); - - return { - tbl_id: base.tbl_id || source?.tbl_id || original?.data?.[originalTraceNum]?.tbl_id || tbl_id, - mappings - }; -} - -function applyCanonicalTableMappings(trace={}, fireflyTrace={}, tableInfo={}) { - if (tableInfo.tbl_id) set(trace, 'tbl_id', tableInfo.tbl_id); - Object.entries(tableInfo.mappings?.data || {}).forEach(([key, value]) => { - if (value !== undefined) set(trace, key, `tables::${value}`); - }); - Object.entries(tableInfo.mappings?.fireflyData || {}).forEach(([key, value]) => { - if (value !== undefined) set(fireflyTrace, key, `tables::${value}`); - }); } export function newTraceFrom(data, selIndexes, newTraceProps, traceAnnotations) { @@ -773,8 +706,10 @@ export function handleBigInt(v) { * @param {string} p.chartId * @param {object[]} p.data * @param {object[]} p.fireflyData + * @param {boolean} [p.replaceTableSources=false] replace and reload the complete table-source set + * @param {boolean} [p.syncExistingSources=false] reconnect and refresh retained table sources */ -export function handleTableSourceConnections({chartId, data, fireflyData}) { +export function handleTableSourceConnections({chartId, data, fireflyData, replaceTableSources=false, syncExistingSources=false}) { const tablesources = makeTableSources(chartId, data, fireflyData); const {tablesources:oldTablesources=[], activeTrace, curveNumberMap=[]} = getChartData(chartId); @@ -782,9 +717,23 @@ export function handleTableSourceConnections({chartId, data, fireflyData}) { // existing watchers for those updates; only reconnect on source changes. const hasTablesources = Array.isArray(tablesources) && tablesources.find((ts) => ts?.tbl_id && !isEmpty(ts.mappings)); - if (!hasTablesources) return; + if (!hasTablesources) { + oldTablesources.forEach((traceTS, idx) => { + if (isEmpty(traceTS)) return; + if (!traceTS._cancel) traceTS._cancel = setupTableWatcher(chartId, traceTS, idx); + if (syncExistingSources) updateChartData(chartId, idx, traceTS); + }); + return; + } + + const numTraces = replaceTableSources + ? tablesources.length + : Math.max(tablesources.length, oldTablesources.length); + + if (replaceTableSources) { + oldTablesources.slice(numTraces).forEach((traceTS) => traceTS?._cancel?.()); + } - const numTraces = Math.max(tablesources.length, oldTablesources.length); range(numTraces).forEach( (idx) => { // range instead of for-loop is to avoid the idx+1 JS's closure problem let traceTS = tablesources[idx]; const oldTraceTS = oldTablesources[idx] || {}; @@ -802,7 +751,7 @@ export function handleTableSourceConnections({chartId, data, fireflyData}) { } } - if (!tablesourcesEqual(traceTS, oldTraceTS)) { + if (replaceTableSources || !tablesourcesEqual(traceTS, oldTraceTS)) { if (oldTraceTS && oldTraceTS._cancel) { oldTraceTS._cancel(); // cancel the previous watcher if exists oldTraceTS._cancel = undefined; @@ -1405,16 +1354,17 @@ function scatterOrHeatmap({tbl_id, xCol, yCol, xOptions}) { export function getSelIndexes(data, selectInfoCls, traceIdx) { return Array.from(selectInfoCls.getSelected()).map((rowIdx) => { - let ptIdx = getPointIdx(data[traceIdx], rowIdx); + let rowTraceIdx = traceIdx; + let ptIdx = getPointIdx(data[rowTraceIdx], rowIdx); if (ptIdx < 0) { data.some((t, idx) => { ptIdx = getPointIdx(t, rowIdx); - traceIdx = idx; + rowTraceIdx = idx; return ptIdx > -1; }); } - return [ptIdx, traceIdx]; - }); + return [ptIdx, rowTraceIdx]; + }).filter(([ptIdx]) => ptIdx >= 0); } @@ -1462,39 +1412,9 @@ function hasNoXY(type, tablesource) { export function getChartProps(chartId, tbl_id, activeTrace) { const {data, layout, fireflyLayout, tablesources, fireflyData, ...rest} = getChartData(chartId) || {}; activeTrace = activeTrace ?? rest.activeTrace ?? 0; - const activeTablesource = get(tablesources, [activeTrace]); - - //Group changes temporarily reset table sources; recover table ID and mappings - //when grouped or fetched traces no longer retain them - const canonicalTable = getCanonicalTableInfo({ - data, - fireflyData, - tablesources, - original: rest._original, - activeTrace, - tbl_id: tbl_id - }); - tbl_id = tbl_id || activeTablesource?.tbl_id || canonicalTable?.tbl_id; - - const canonicalMappings = { - ...(canonicalTable?.mappings?.data || {}), - ...Object.entries(canonicalTable?.mappings?.fireflyData || {}) - .reduce((p, [key, value]) => set(p, `fireflyData.${activeTrace}.${key}`, value), {}) - }; - - - const activeMappings = Object.fromEntries( - Object.entries(activeTablesource?.mappings || {}) - .filter(([, value]) => value !== undefined && value !== '') - ); - - //Prefer current/active non-empty mappings, falling back to canonical/original mappings - const mappings = {...canonicalMappings, ...activeMappings}; - - //Rebuild a usable table source while preserving existing source behavior - const tablesource = (activeTablesource || tbl_id) - ? {...(activeTablesource || {}), tbl_id, mappings} - : undefined; + const tablesource = get(tablesources, [activeTrace], tbl_id && {tbl_id}); + tbl_id = tbl_id || tablesource?.tbl_id; + const mappings = tablesource?.mappings; const multiTrace = activeTrace > 0 || data?.length > 1; const type = get(data, `${activeTrace}.type`, 'scatter'); const dataType = fireflyData?.[activeTrace]?.dataType; @@ -1517,8 +1437,7 @@ export function getChartProps(chartId, tbl_id, activeTrace) { export function getTblIdFromChart(chartId, traceNum) { const {data, fireflyData, tablesources, activeTrace} = getChartData(chartId) || {}; traceNum = traceNum ?? activeTrace; - return tablesources?.[traceNum]?.tbl_id || data?.[traceNum]?.tbl_id || fireflyData?.[traceNum]?.tbl_id || - fireflyData?.[traceNum]?.groupBy?.baseTable?.tbl_id || fireflyData?.[traceNum]?.baseTable?.tbl_id; + return tablesources?.[traceNum]?.tbl_id || data?.[traceNum]?.tbl_id || fireflyData?.[traceNum]?.tbl_id; } export function hasTracesFromSameTable(chartId) { diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index fd05fbf61f..69e60ba27e 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -130,13 +130,14 @@ export function dispatchChartTraceRemove(chartId, traceNum, dispatcher= flux.pro * @param {Object} p - dispatch parameetrs * @param {string} p.chartId - chart id * @param {Object} p.changes - object with the path-string keys and values of the changed props + * @param {boolean} [p.replaceTableSources=false] - replace and reload the complete table-source set * @param {Function} [p.dispatcher=flux.process] - only for special dispatching uses such as remote * @public * @function dispatchChartUpdate * @memberof firefly.action */ -export function dispatchChartUpdate({chartId, changes, dispatcher=flux.process}) { - dispatcher({type: CHART_UPDATE, payload: {chartId, changes}}); +export function dispatchChartUpdate({chartId, changes, replaceTableSources=false, dispatcher=flux.process}) { + dispatcher({type: CHART_UPDATE, payload: {chartId, changes, replaceTableSources}}); } /** @@ -324,7 +325,7 @@ function chartTraceRemove(action) { function chartUpdate(action) { return (dispatch) => { - const {chartId, changes} = action.payload; + const {chartId, changes, replaceTableSources=false} = action.payload; // when selection is undefined, selections layer must be removed if (changes.hasOwnProperty('selection') && !changes.selection) changes['layout.selections'] = []; // remove any table's mappings from changes because it will be applied by the connectors. @@ -340,7 +341,7 @@ function chartUpdate(action) { // lazy table connection const {mounted} = getChartData(chartId); if (mounted > 0 && (!isEmpty(data) || !isEmpty(fireflyData))) { - handleTableSourceConnections({chartId, data, fireflyData}); + handleTableSourceConnections({chartId, data, fireflyData, replaceTableSources}); } }; } diff --git a/src/firefly/js/charts/ui/PlotlyChartArea.jsx b/src/firefly/js/charts/ui/PlotlyChartArea.jsx index a04c82e08c..d7e00466e9 100644 --- a/src/firefly/js/charts/ui/PlotlyChartArea.jsx +++ b/src/firefly/js/charts/ui/PlotlyChartArea.jsx @@ -23,7 +23,7 @@ export function PlotlyChartArea({chartId, widthPx, heightPx, thumbnail}) { useEffect(()=> { const {fireflyData} = getChartData(chartId); - handleTableSourceConnections({chartId, data, fireflyData}); + handleTableSourceConnections({chartId, data, fireflyData, syncExistingSources: true}); return () => { if (getChartData(chartId)?.mounted === 0) { clearChartConn({chartId}); diff --git a/src/firefly/js/charts/ui/options/ScatterOptions.jsx b/src/firefly/js/charts/ui/options/ScatterOptions.jsx index 2d0fb72473..0663114409 100644 --- a/src/firefly/js/charts/ui/options/ScatterOptions.jsx +++ b/src/firefly/js/charts/ui/options/ScatterOptions.jsx @@ -5,7 +5,7 @@ import {Stack, Typography} from '@mui/joy'; import {Expression} from '../../../util/expr/Expression.js'; import {dispatchChartUpdate, getChartData, hasUpperLimits} from '../../ChartsCntlr.js'; import { - clearChartConn, getChartProps, getGroupableColumns, getGroupByColumn, getMinScatterGLRows, GROUP_BY_NONE, + getChartProps, getGroupableColumns, getGroupByColumn, getMinScatterGLRows, GROUP_BY_NONE, isGroupByAllowed, isGroupedChart, makeScatterGroupByChanges } from '../../ChartUtil.js'; import {FieldGroup} from '../../../ui/FieldGroup.jsx'; @@ -257,8 +257,7 @@ export function submitChangesScatter({chartId, activeTrace, fields, tbl_id, rend const groupedFields = omit(changes, groupByKey); const evalChanges = evalChangesFromFields(chartId, tbl_id, groupedFields); const groupChanges = makeScatterGroupByChanges({chartId, tbl_id, activeTrace, groupByColumn, changes: evalChanges}); - clearChartConn({chartId}); - dispatchChartUpdate({chartId, changes: groupChanges}); + dispatchChartUpdate({chartId, changes: groupChanges, replaceTableSources: true}); dispatchComponentStateChange(TRACE_OPTIONS_COMPONENT_KEY, {isOpen: Boolean(groupByColumn)}); return; } diff --git a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx index 90452ca628..569dee76eb 100644 --- a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx +++ b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx @@ -259,23 +259,24 @@ export const applyUnitConversion = ({fireflyData, data, inFields, axisType, newU export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, renderTreeId}) { const {data, fireflyData} = getChartData(chartId); const {spectralAxis={}, fluxAxis={}} = getSpectrumDM(getTblById(tbl_id)) || {}; - const chartProps = getChartProps(chartId, tbl_id, activeTrace); - - //The table source can be briefly unavailable while grouped traces are being rebuilt. Keep all - //canonical table mappings in the submit payload so a trace options change does not accidntally clear - //the spectral axes or their error/limit columns - Object.entries(chartProps.mappings || {}).forEach(([key, value]) => { - const fieldKey = key.startsWith('fireflyData.') - ? `_tables.${key}` - : `_tables.data.${activeTrace}.${key}`; - if (!fields[fieldKey] && value) fields = updateSet(fields, [fieldKey], value); + // A grouped spectrum's generated traces share the same axes. Preserve each trace's mappings so + // changing one trace's style cannot leave the next selected trace with a partial table source. + const tracesToPreserve = isGroupedChart(chartId) ? range(data.length) : [activeTrace]; + tracesToPreserve.forEach((traceNum) => { + const {mappings={}} = getChartProps(chartId, tbl_id, traceNum); + Object.entries(mappings).forEach(([key, value]) => { + const fieldKey = key.startsWith('fireflyData.') + ? `_tables.${key}` + : `_tables.data.${traceNum}.${key}`; + if (!fields[fieldKey] && value) fields = updateSet(fields, [fieldKey], value); + }); + if (!fields[`_tables.data.${traceNum}.x`] && spectralAxis.value) { + fields = updateSet(fields, [`_tables.data.${traceNum}.x`], spectralAxis.value); + } + if (!fields[`_tables.data.${traceNum}.y`] && fluxAxis.value) { + fields = updateSet(fields, [`_tables.data.${traceNum}.y`], fluxAxis.value); + } }); - if (!fields[`_tables.data.${activeTrace}.x`] && spectralAxis.value) { - fields = updateSet(fields, [`_tables.data.${activeTrace}.x`], spectralAxis.value); - } - if (!fields[`_tables.data.${activeTrace}.y`] && fluxAxis.value) { - fields = updateSet(fields, [`_tables.data.${activeTrace}.y`], fluxAxis.value); - } // get units and spectral frame options from the fields of active trace const xUnit = fields[`fireflyData.${activeTrace}.xUnit`]; From 378eb8557e31c6a00884f289a3223614d6b77d57 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Sun, 9 Aug 2026 20:11:33 -0700 Subject: [PATCH 04/11] fixed more bugs --- src/firefly/js/charts/ChartUtil.js | 125 +++++++++++++++--- src/firefly/js/charts/TableStatsCntlr.js | 13 +- .../js/charts/dataTypes/FireflyGenericData.js | 9 +- .../js/charts/dataTypes/FireflySpectrum.js | 7 +- .../js/charts/ui/PinnedChartContainer.jsx | 8 +- src/firefly/js/charts/ui/PlotlyWrapper.jsx | 5 +- 6 files changed, 136 insertions(+), 31 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 744a2447c6..a6475af19a 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -310,7 +310,8 @@ export function combineAllTraceFrom(chartId, selIndexes, newTraceProps) { const combine = (flatTarget, source) => { Object.entries(flatTarget).forEach( ([key, value]) => { if (Array.isArray(value)) { - value.push(...get(source, key)); + const sourceValue = get(source, key); + if (Array.isArray(sourceValue)) value.push(...sourceValue); } }); return flatTarget; @@ -381,10 +382,14 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group const fireflyData = chartData.fireflyData || []; const baseData = simpleCloneDeep(data[activeTrace] || {}); const baseFirefly = simpleCloneDeep(fireflyData[activeTrace] || {}); + const activeTablesource = chartData.tablesources?.[activeTrace] || {}; + const baseState = baseFirefly?.groupBy?.baseState + ? simpleCloneDeep(baseFirefly.groupBy.baseState) + : makeGroupByBaseState(baseData, baseFirefly, activeTablesource, activeTrace, tbl_id); restoreTableSourceMappings({ trace: baseData, fireflyTrace: baseFirefly, - tablesource: chartData.tablesources?.[activeTrace], + tablesource: activeTablesource, traceNum: activeTrace, tbl_id }); @@ -399,11 +404,20 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group }; if (!groupByColumn) { - clearGroupBy(baseData, baseFirefly); + const ungroupedData = simpleCloneDeep(baseState.data); + const ungroupedFirefly = simpleCloneDeep(baseState.fireflyData); + clearGroupBy(ungroupedData, ungroupedFirefly); + restoreTableSourceMappings({ + trace: ungroupedData, + fireflyTrace: ungroupedFirefly, + tablesource: {tbl_id: baseState.tbl_id, mappings: baseState.mappings}, + traceNum: baseState.traceNum, + tbl_id + }); return { ...commonChanges, - data: [baseData], - fireflyData: [baseFirefly], + data: [ungroupedData], + fireflyData: [ungroupedFirefly], activeTrace: 0, curveNumberMap: makeCurveNumberMap(1, 0), 'layout.showlegend': undefined, @@ -416,11 +430,20 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group .map((v) => String(v ?? '').trim().replace(/^'(.*)'$/, '$1')) .filter((v) => v !== ''); if (!groupCol || groupValues.length === 0) { - clearGroupBy(baseData, baseFirefly); + const ungroupedData = simpleCloneDeep(baseState.data); + const ungroupedFirefly = simpleCloneDeep(baseState.fireflyData); + clearGroupBy(ungroupedData, ungroupedFirefly); + restoreTableSourceMappings({ + trace: ungroupedData, + fireflyTrace: ungroupedFirefly, + tablesource: {tbl_id: baseState.tbl_id, mappings: baseState.mappings}, + traceNum: baseState.traceNum, + tbl_id + }); return { ...commonChanges, - data: [baseData], - fireflyData: [baseFirefly], + data: [ungroupedData], + fireflyData: [ungroupedFirefly], activeTrace: 0, curveNumberMap: makeCurveNumberMap(1, 0), 'layout.showlegend': undefined, @@ -446,7 +469,8 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group fd.groupBy = { column: groupCol.name, value, - baseTraceStyle: getGroupByBaseStyle(baseData) + baseTraceStyle: getGroupByBaseStyle(baseData), + baseState: simpleCloneDeep(baseState) }; fd.filters = makeGroupFilter(groupCol.name, value); @@ -466,6 +490,24 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group }; } +function makeGroupByBaseState(trace, fireflyTrace, tablesource, traceNum, tbl_id) { + const data = simpleCloneDeep(trace); + const fd = simpleCloneDeep(fireflyTrace); + + // Older automatically-grouped spectra do not yet carry an ungrouped snapshot. + if (fd?.groupBy) { + clearGroupBy(data, fd); + if (data.firefly) delete data.firefly.rowIdx; + } + return { + data, + fireflyData: fd, + tbl_id: tablesource?.tbl_id || data.tbl_id || fd.tbl_id || tbl_id, + mappings: simpleCloneDeep(tablesource?.mappings || {}), + traceNum + }; +} + function makeCurveNumberMap(traceCount, activeTrace) { return [...range(traceCount).filter((idx) => idx !== activeTrace), activeTrace]; } @@ -480,6 +522,8 @@ function clearGroupBy(trace, fireflyTrace) { restoreGroupByBaseStyle(trace, fireflyTrace); if (fireflyTrace?.groupBy) { delete trace.name; + delete trace.legendgroup; + delete trace.showlegend; } if (fireflyTrace) { delete fireflyTrace.groupBy; @@ -715,11 +759,11 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac // Partial data updates contain arrays, not table-source metadata. Keep the // existing watchers for those updates; only reconnect on source changes. - const hasTablesources = Array.isArray(tablesources) && tablesources.find((ts) => - ts?.tbl_id && !isEmpty(ts.mappings)); - if (!hasTablesources) { + const hasTableSourceUpdates = Array.isArray(tablesources) && tablesources.some((ts) => + ts?.tbl_id && (!isEmpty(ts.mappings) || typeof ts.fetchData === 'function')); + if (!hasTableSourceUpdates) { oldTablesources.forEach((traceTS, idx) => { - if (isEmpty(traceTS)) return; + if (!traceTS?.tbl_id) return; if (!traceTS._cancel) traceTS._cancel = setupTableWatcher(chartId, traceTS, idx); if (syncExistingSources) updateChartData(chartId, idx, traceTS); }); @@ -767,6 +811,7 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac //creates a new one.. and save the cancel handle if (doUpdate) { // fetch data syncs highlighted and selected with the table + traceTS._sourceId = uniqueId('chart-source-'); updateChartData(chartId, idx, traceTS); } else { if (idx === activeTrace && isFullyLoaded(traceTS.tbl_id)) { @@ -830,6 +875,17 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // make sure the chart is not yet removed if (isEmpty(chartData) || !chartData?.mounted) { return; } + // A table event may already be queued when its trace source is replaced. + // A filter is table-global, however, so apply it using the current source. + const currentSource = chartData.tablesources?.[traceNum]; + const sourceWasReplaced = action.type && tablesource._sourceId && currentSource?._sourceId && + tablesource._sourceId !== currentSource._sourceId; + const isFilterEvent = action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER; + if (sourceWasReplaced) { + if (!isFilterEvent) return; + tablesource = currentSource; + } + // Only Scatter Plot does update on a table sort event. if (action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_SORT) { const traceType = get(chartData, ['data', traceNum, 'type'], 'scatter'); @@ -858,7 +914,7 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // Mark the trace loading while its type-specific fetch obtains the // current table data (and, for grouped traces, its group subset). const changes = getDataChangesForMappings({mappings, traceNum}); - if (action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER) { + if (isFilterEvent) { changes['layout.xaxis.autorange'] = true; changes['layout.xaxis.range'] = undefined; changes['layout.yaxis.autorange'] = true; @@ -876,7 +932,7 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // fetch data for both Firefly recognized or unrecognized plotly chart types if (tablesource.fetchData) { - const fetchSource = action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER + const fetchSource = isFilterEvent ? {...tablesource, resetAxes: true} : tablesource; tablesource.fetchData(chartId, traceNum, fetchSource); @@ -896,10 +952,7 @@ function makeTableSources(chartId, data=[], fireflyData=[]) { const currentData = (data.length < fireflyData.length) ? fireflyData : data; return currentData.map((d, traceNum) => { - const fireflyDataFlatten = flattenObject(fireflyData[traceNum] || {}, `fireflyData.${traceNum}`); - // fireflyData mappings have full path - const flattenData = assign(flattenObject(data[traceNum] || {}), fireflyDataFlatten); - const ds = data[traceNum] ? {mappings: getTableSourceMappings(flattenData)} : {}; //avoid flattening arrays + const ds = data[traceNum] ? {mappings: getTraceTableSourceMappings(data[traceNum], fireflyData[traceNum], traceNum)} : {}; //avoid flattening arrays // table id can be a part of fireflyData too const tbl_id = get(data, `${traceNum}.tbl_id`) || get(fireflyData, `${traceNum}.tbl_id`); @@ -919,6 +972,31 @@ function makeTableSources(chartId, data=[], fireflyData=[]) { }); } +function getTraceTableSourceMappings(dataTrace, fireflyTrace, traceNum) { + const fireflyDataFlatten = flattenObject(fireflyTrace || {}, `fireflyData.${traceNum}`); + // fireflyData mappings have full path + const flattenData = assign(flattenObject(dataTrace || {}), fireflyDataFlatten); + return getTableSourceMappings(flattenData); +} + +function getBestTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}) { + const activeMappings = tablesources?.[activeTrace]?.mappings; + if (!isEmpty(activeMappings)) return activeMappings; + + const sameTableSource = tablesources?.find((ts) => + ts?.tbl_id && ts.tbl_id === tbl_id && !isEmpty(ts.mappings)); + if (sameTableSource) return sameTableSource.mappings; + + const baseState = fireflyData?.[activeTrace]?.groupBy?.baseState; + if (!isEmpty(baseState?.mappings)) return baseState.mappings; + + const sameTableBaseState = fireflyData?.find((fd) => + fd?.groupBy?.baseState?.tbl_id === tbl_id && !isEmpty(fd.groupBy.baseState.mappings)); + if (sameTableBaseState) return sameTableBaseState.groupBy.baseState.mappings; + + return getTraceTableSourceMappings(data?.[activeTrace], fireflyData?.[activeTrace], activeTrace); +} + function getTraceTSEntries({chartDataType, traceTS, chartId, traceNum}) { if (chartDataType === 'fireflyHistogram') { return histogramTSGetter({traceTS, chartId, traceNum}); @@ -1412,9 +1490,12 @@ function hasNoXY(type, tablesource) { export function getChartProps(chartId, tbl_id, activeTrace) { const {data, layout, fireflyLayout, tablesources, fireflyData, ...rest} = getChartData(chartId) || {}; activeTrace = activeTrace ?? rest.activeTrace ?? 0; - const tablesource = get(tablesources, [activeTrace], tbl_id && {tbl_id}); - tbl_id = tbl_id || tablesource?.tbl_id; - const mappings = tablesource?.mappings; + const activeTablesource = get(tablesources, [activeTrace]); + tbl_id = tbl_id || activeTablesource?.tbl_id || data?.[activeTrace]?.tbl_id || fireflyData?.[activeTrace]?.tbl_id; + const mappings = getBestTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}); + const tablesource = (activeTablesource || tbl_id) + ? {...(activeTablesource || {}), tbl_id, mappings} + : undefined; const multiTrace = activeTrace > 0 || data?.length > 1; const type = get(data, `${activeTrace}.type`, 'scatter'); const dataType = fireflyData?.[activeTrace]?.dataType; diff --git a/src/firefly/js/charts/TableStatsCntlr.js b/src/firefly/js/charts/TableStatsCntlr.js index a32d37ebed..ead5ed3a36 100644 --- a/src/firefly/js/charts/TableStatsCntlr.js +++ b/src/firefly/js/charts/TableStatsCntlr.js @@ -27,6 +27,8 @@ export const TBLSTATS_DATA_KEY = 'tblstats'; export const LOAD_TBL_STATS = `${TBLSTATS_DATA_KEY}/LOAD_TBL_STATS`; export const UPDATE_TBL_STATS = `${TBLSTATS_DATA_KEY}/UPDATE_TBL_STATS`; +const statsInFlight = new Set(); + export default {actionCreators, reducers}; @@ -58,6 +60,8 @@ export function dispatchLoadTblStats(searchRequest, dispatcher= flux.process) { if (resultSetID !== resultSetIDNow || curNumericColCnt !== numericColCnt) { // also reload stats if the number of numeric columns changes. + if (statsInFlight.has(tbl_id)) return; + statsInFlight.add(tbl_id); dispatcher({type: LOAD_TBL_STATS, payload: {searchRequest}}); } } @@ -196,7 +200,14 @@ function fetchTblStats(dispatch, activeTableServerRequest) { colStats: undefined })); } - ); + ).finally(() => { + statsInFlight.delete(tbl_id); + const currentRequest = getTblById(tbl_id)?.request; + const currentResultSetID = get(getTblById(tbl_id), 'tableMeta.resultSetID'); + if (currentRequest && currentResultSetID !== resultSetIDOnSubmit) { + dispatchLoadTblStats(currentRequest); + } + }); } export function getColValStats(tblId) { diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index 84ca45bbb3..6c00df176e 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -1,7 +1,7 @@ /* * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ -import {get, isArray, isUndefined, uniqueId, isNil} from 'lodash'; +import {get, isArray, isEmpty, isUndefined, uniqueId, isNil} from 'lodash'; import {getTblById, getColumns, getColumn, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; import {cloneRequest, makeSubQueryRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {getFiltersAsSql} from '../../tables/FilterInfo.js'; @@ -25,7 +25,7 @@ const numberOrArrayProps = ['marker.size']; export function getTraceTSEntries({traceTS, chartId, traceNum}) { const {mappings} = traceTS || {}; - if (mappings) { + if (!isEmpty(mappings)) { const options = Object.assign({}, mappings); if (hasUpperLimits(chartId, traceNum) || hasLowerLimits(chartId, traceNum)) { @@ -78,7 +78,10 @@ async function fetchData(chartId, traceNum, tablesource) { dispatchError(chartId, traceNum, reason); }); - if (tableModel.error) { + // Ignore a fetch only when both source generations are known and differ. + const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; + if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; + if (tableModel?.error) { return dispatchError(chartId, traceNum, tableModel.error); } diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index e24d0ea13b..d5a211f4e3 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -5,7 +5,7 @@ import {isEmpty, pickBy, cloneDeep, set, uniqueId} from 'lodash'; import {getTblById, getColumn, doFetchTable, getColumnIdx} from '../../tables/TableUtil.js'; import {getSpectrumDM, REF_POS} from '../../voAnalyzer/SpectrumDM.js'; -import {dispatchChartUpdate, dispatchError} from '../ChartsCntlr.js'; +import {dispatchChartUpdate, dispatchError, getChartData} from '../ChartsCntlr.js'; import {getDataChangesForMappings, getOriginalRowIndexes, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; import {addOtherChanges, createChartTblRequest, getTraceTSEntries as genericTSGetter} from './FireflyGenericData.js'; @@ -51,7 +51,10 @@ async function fetchData(chartId, traceNum, tablesource) { dispatchError(chartId, traceNum, reason); }); - if (tableModel.error) { + // Ignore a fetch only when both source generations are known and differ. + const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; + if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; + if (tableModel?.error) { return dispatchError(chartId, traceNum, tableModel.error); } diff --git a/src/firefly/js/charts/ui/PinnedChartContainer.jsx b/src/firefly/js/charts/ui/PinnedChartContainer.jsx index e3609cc009..5c9b066fcb 100644 --- a/src/firefly/js/charts/ui/PinnedChartContainer.jsx +++ b/src/firefly/js/charts/ui/PinnedChartContainer.jsx @@ -180,8 +180,12 @@ export function ChartBadgeLabel({labelStr}) { function doPinChart({chartId, autoLayout=true, displayPinMessage=true }) { - const chartData = cloneDeep(omit(getChartData(chartId), ['_original', 'mounted'])); - chartData?.tablesources?.forEach((ts) => Reflect.deleteProperty(ts, '_cancel')); + const chartData = cloneDeep(omit(getChartData(chartId), ['_original', 'mounted', 'selection'])); + if (chartData?.layout) delete chartData.layout.selections; + chartData?.tablesources?.forEach((ts) => { + Reflect.deleteProperty(ts, '_cancel'); + Reflect.deleteProperty(ts, '_sourceId'); + }); const pinnedCnt = getViewerItemIds(getMultiViewRoot(), PINNED_CHART_VIEWER_ID)?.length ?? 0; if (pinnedCnt >= PINNED_MAX) { diff --git a/src/firefly/js/charts/ui/PlotlyWrapper.jsx b/src/firefly/js/charts/ui/PlotlyWrapper.jsx index 9a01ee87a1..e9f66cf71b 100644 --- a/src/firefly/js/charts/ui/PlotlyWrapper.jsx +++ b/src/firefly/js/charts/ui/PlotlyWrapper.jsx @@ -344,6 +344,9 @@ export class PlotlyWrapper extends Component { const {layout} = getChartData(chartId); if (layout && !this.props.thumbnail) { Object.entries(changes).forEach( ([k, v]) => { + // Plotly emits private selection edit paths such as selections[0].yref. + // Selection state is persisted explicitly by PlotlyChartArea. + if (k.startsWith('selections[')) return; if (k === 'xaxis' && Array.isArray(v)) { set(layout, 'xaxis.range', v); set(layout, 'xaxis.autorange', false); @@ -432,4 +435,4 @@ PlotlyWrapper.propTypes = { autoDetectResizing : PropTypes.bool, doingResize: PropTypes.bool, thumbnail: PropTypes.bool -}; \ No newline at end of file +}; From 0d82f3ce416c41c8c70f91e2e5f38b9457c1adf5 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Mon, 10 Aug 2026 15:18:36 -0700 Subject: [PATCH 05/11] more fixes --- src/firefly/js/charts/ChartUtil.js | 13 ++++++-- .../js/charts/ui/options/BasicOptions.jsx | 30 ++++++++++++------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index a6475af19a..8f5faf6227 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -979,7 +979,7 @@ function getTraceTableSourceMappings(dataTrace, fireflyTrace, traceNum) { return getTableSourceMappings(flattenData); } -function getBestTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}) { +function getGenericTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}) { const activeMappings = tablesources?.[activeTrace]?.mappings; if (!isEmpty(activeMappings)) return activeMappings; @@ -987,6 +987,10 @@ function getBestTableSourceMappings({data, fireflyData, tablesources, activeTrac ts?.tbl_id && ts.tbl_id === tbl_id && !isEmpty(ts.mappings)); if (sameTableSource) return sameTableSource.mappings; + return getTraceTableSourceMappings(data?.[activeTrace], fireflyData?.[activeTrace], activeTrace); +} + +function getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}) { const baseState = fireflyData?.[activeTrace]?.groupBy?.baseState; if (!isEmpty(baseState?.mappings)) return baseState.mappings; @@ -994,7 +998,7 @@ function getBestTableSourceMappings({data, fireflyData, tablesources, activeTrac fd?.groupBy?.baseState?.tbl_id === tbl_id && !isEmpty(fd.groupBy.baseState.mappings)); if (sameTableBaseState) return sameTableBaseState.groupBy.baseState.mappings; - return getTraceTableSourceMappings(data?.[activeTrace], fireflyData?.[activeTrace], activeTrace); + return {}; } function getTraceTSEntries({chartDataType, traceTS, chartId, traceNum}) { @@ -1492,7 +1496,10 @@ export function getChartProps(chartId, tbl_id, activeTrace) { activeTrace = activeTrace ?? rest.activeTrace ?? 0; const activeTablesource = get(tablesources, [activeTrace]); tbl_id = tbl_id || activeTablesource?.tbl_id || data?.[activeTrace]?.tbl_id || fireflyData?.[activeTrace]?.tbl_id; - const mappings = getBestTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}); + const genericMappings = getGenericTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}); + const mappings = !isEmpty(genericMappings) + ? genericMappings + : getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}); const tablesource = (activeTablesource || tbl_id) ? {...(activeTablesource || {}), tbl_id, mappings} : undefined; diff --git a/src/firefly/js/charts/ui/options/BasicOptions.jsx b/src/firefly/js/charts/ui/options/BasicOptions.jsx index 13f54d3c16..ba8af7d738 100644 --- a/src/firefly/js/charts/ui/options/BasicOptions.jsx +++ b/src/firefly/js/charts/ui/options/BasicOptions.jsx @@ -106,6 +106,12 @@ export function BasicOptions({activeTrace:pActiveTrace, tbl_id:ptbl_id, chartId, ); } +function isCurrentAxisMapping({chartId, activeTrace, axis, value}) { + if (isUndefined(value)) return false; + const {mappings={}} = getChartProps(chartId, undefined, activeTrace); + return String(value).trim() === String(mappings?.[axis] ?? '').trim(); +} + export function basicFieldReducer({chartId, activeTrace}) { return (inFields, action) => { @@ -118,17 +124,19 @@ export function basicFieldReducer({chartId, activeTrace}) { fieldKey = get(action.payload, 'fieldKey'); ['x','y'].forEach((a) => { if (fieldKey === `_tables.data.${activeTrace}.${a}`) { // column name or expression changed - // unset the axis title so that the chart generates a title based on the changed column name - // but not in spectrum because spectrumReducer changes axes labels itself which need to be persisted - if (!isSpectrum(chartId)) inFields = updateSet(inFields, [`layout.${a}axis.title.text`, 'value'], undefined); - - inFields = updateSet(inFields, [`fireflyLayout.${a}axis.min`, 'value'], undefined); - inFields = updateSet(inFields, [`fireflyLayout.${a}axis.max`, 'value'], undefined); - inFields = updateSet(inFields, [`__${a}reset`, 'value'], 'true'); - const optFldName = `__${a}options`; - const currOptions = get(inFields, [optFldName, 'value']); - // do not reset grid selection - inFields = updateSet(inFields, [optFldName, 'value'], filterOptions(currOptions, ['grid', 'opposite'])); + if (!isCurrentAxisMapping({chartId, activeTrace, axis: a, value: get(action.payload, 'value')})) { + // unset the axis title so that the chart generates a title based on the changed column name + // but not in spectrum because spectrumReducer changes axes labels itself which need to be persisted + if (!isSpectrum(chartId)) inFields = updateSet(inFields, [`layout.${a}axis.title.text`, 'value'], undefined); + + inFields = updateSet(inFields, [`fireflyLayout.${a}axis.min`, 'value'], undefined); + inFields = updateSet(inFields, [`fireflyLayout.${a}axis.max`, 'value'], undefined); + inFields = updateSet(inFields, [`__${a}reset`, 'value'], 'true'); + const optFldName = `__${a}options`; + const currOptions = get(inFields, [optFldName, 'value']); + // do not reset grid selection + inFields = updateSet(inFields, [optFldName, 'value'], filterOptions(currOptions, ['grid', 'opposite'])); + } } }); } From 9fd1a41dc0e8d2af4f557e751bc1707009382e14 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Mon, 10 Aug 2026 18:51:55 -0700 Subject: [PATCH 06/11] fixes continued --- src/firefly/js/charts/ChartUtil.js | 27 +++++++++---------- .../js/charts/ui/options/ScatterOptions.jsx | 10 +++---- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 8f5faf6227..70d52beb20 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -23,11 +23,7 @@ import {TABLE_FILTER, TABLE_HIGHLIGHT, TABLE_LOADED, TABLE_SELECT, TABLE_SORT} f import {getSpectrumDM} from '../voAnalyzer/SpectrumDM.js'; import {findTableCenterColumns} from '../voAnalyzer/TableAnalysis.js'; import {dispatchLoadTblStats, getColValStats} from './TableStatsCntlr.js'; -import { - dispatchChartHighlighted, - dispatchChartSelect, - dispatchChartUpdate, - dispatchSetActiveTrace, +import {dispatchChartHighlighted, dispatchChartSelect, dispatchChartUpdate, dispatchSetActiveTrace, getChartData } from './ChartsCntlr.js'; import {Expression} from '../util/expr/Expression.js'; @@ -37,8 +33,7 @@ import {SelectInfo} from '../tables/SelectInfo.js'; import {getTraceTSEntries as histogramTSGetter} from './dataTypes/FireflyHistogram.js'; import {getTraceTSEntries as heatmapTSGetter} from './dataTypes/FireflyHeatmap.js'; import {getTraceTSEntries as genericTSGetter} from './dataTypes/FireflyGenericData.js'; -import { - getTraceTSEntries as spectrumTSGetter, spectrumPlot, spectrumType +import {getTraceTSEntries as spectrumTSGetter, spectrumPlot, spectrumType } from './dataTypes/FireflySpectrum.js'; import {toRGBA as colorToRGBA} from '../util/Color.js'; import {MetaConst} from '../data/MetaConst'; @@ -70,7 +65,6 @@ export const HIGHLIGHTED_PROPS = { const FSIZE = 12; export const TBL_SRC_PATTERN = /^tables::(.+)/; -export const GROUP_BY_NONE = ''; /** * Extract table mappings from flattened chart data. @@ -349,7 +343,7 @@ export function combineAllTraceFrom(chartId, selIndexes, newTraceProps) { //Chart "Group By" logic helpers below export function getGroupByColumn(chartId) { const {fireflyData=[]} = getChartData(chartId) || {}; - return fireflyData.find((fd) => fd?.groupBy?.column)?.groupBy.column || GROUP_BY_NONE; + return fireflyData.find((fd) => fd?.groupBy?.column)?.groupBy.column || ''; } export function isGroupedChart(chartId) { @@ -374,7 +368,7 @@ export function getGroupableColumns(tbl_id) { .map((c) => ({...c})); } -export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, groupByColumn=GROUP_BY_NONE, changes={}}) { +export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, groupByColumn='', changes={}}) { const chartData = simpleCloneDeep(getChartData(chartId, {})); Object.entries(changes).forEach(([key, value]) => set(chartData, key, value)); @@ -420,7 +414,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group fireflyData: [ungroupedFirefly], activeTrace: 0, curveNumberMap: makeCurveNumberMap(1, 0), - 'layout.showlegend': undefined, + 'layout.showlegend': false, 'layout.legend.title.text': undefined }; } @@ -446,7 +440,7 @@ export function makeScatterGroupByChanges({chartId, tbl_id, activeTrace=0, group fireflyData: [ungroupedFirefly], activeTrace: 0, curveNumberMap: makeCurveNumberMap(1, 0), - 'layout.showlegend': undefined, + 'layout.showlegend': false, 'layout.legend.title.text': undefined }; } @@ -810,8 +804,10 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac if (!isEmpty(traceTS)) { //creates a new one.. and save the cancel handle if (doUpdate) { - // fetch data syncs highlighted and selected with the table + //assign a unique id to this table source so queued watcher events from a + //replaced source can be ignored before they update the chart (this led to filtering/pinning related bugs after introducing chart grouping) traceTS._sourceId = uniqueId('chart-source-'); + // fetch data syncs highlighted and selected with the table updateChartData(chartId, idx, traceTS); } else { if (idx === activeTrace && isFullyLoaded(traceTS.tbl_id)) { @@ -875,8 +871,9 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // make sure the chart is not yet removed if (isEmpty(chartData) || !chartData?.mounted) { return; } - // A table event may already be queued when its trace source is replaced. - // A filter is table-global, however, so apply it using the current source. + // Ignore queued watcher events from an older table-source generation + // Table filters are global, so handle them with the current source, + // otherwise a stale source may point to an old grouped trace or old mappings const currentSource = chartData.tablesources?.[traceNum]; const sourceWasReplaced = action.type && tablesource._sourceId && currentSource?._sourceId && tablesource._sourceId !== currentSource._sourceId; diff --git a/src/firefly/js/charts/ui/options/ScatterOptions.jsx b/src/firefly/js/charts/ui/options/ScatterOptions.jsx index 0663114409..f5e0c0a232 100644 --- a/src/firefly/js/charts/ui/options/ScatterOptions.jsx +++ b/src/firefly/js/charts/ui/options/ScatterOptions.jsx @@ -5,7 +5,7 @@ import {Stack, Typography} from '@mui/joy'; import {Expression} from '../../../util/expr/Expression.js'; import {dispatchChartUpdate, getChartData, hasUpperLimits} from '../../ChartsCntlr.js'; import { - getChartProps, getGroupableColumns, getGroupByColumn, getMinScatterGLRows, GROUP_BY_NONE, + getChartProps, getGroupableColumns, getGroupByColumn, getMinScatterGLRows, isGroupByAllowed, isGroupedChart, makeScatterGroupByChanges } from '../../ChartUtil.js'; import {FieldGroup} from '../../../ui/FieldGroup.jsx'; @@ -250,8 +250,8 @@ export function submitChangesScatter({chartId, activeTrace, fields, tbl_id, rend Object.assign(changes, fields); const groupByKey = groupByFieldKey(activeTrace); - const groupByColumn = fields[groupByKey] || GROUP_BY_NONE; - const currentGroupBy = getGroupByColumn(chartId) || GROUP_BY_NONE; + const groupByColumn = fields[groupByKey] || ''; + const currentGroupBy = getGroupByColumn(chartId) || ''; const groupByChanged = groupByColumn !== currentGroupBy; if (chartId && has(fields, groupByKey) && groupByChanged && isGroupByAllowed(chartId, activeTrace)) { const groupedFields = omit(changes, groupByKey); @@ -333,9 +333,9 @@ export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char ({label: label || name, value: name})) ]} {...withDefaults(props)}/>); From bfdf4a7990ea3d68c8851de30a6e13289b712589 Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Mon, 10 Aug 2026 23:49:32 -0700 Subject: [PATCH 07/11] fix axes bug after filtering --- src/firefly/js/charts/ChartUtil.js | 13 +++++++++---- src/firefly/js/charts/ChartsCntlr.js | 7 ++++--- .../js/charts/dataTypes/FireflyGenericData.js | 7 ++++--- src/firefly/js/charts/dataTypes/FireflySpectrum.js | 7 ++++--- 4 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 70d52beb20..207531bfab 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -81,6 +81,11 @@ export function getTableSourceMappings(flattenData={}) { }, {}); } +export function getFilterResetAutorange(axisLayout) { + const {autorange, range=[]} = axisLayout || {}; + return autorange === 'reversed' || range[1] < range[0] ? 'reversed' : true; +} + export function getOriginalRowIndexes(tableModel) { const origIdx = getColumnIdx(tableModel, 'ORIG_IDX'); return origIdx >= 0 ? tableModel?.tableData?.data?.map((row) => row[origIdx]) : undefined; @@ -908,13 +913,13 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // may not need to call it as often. if this becomes a performance issue, then optimize. dispatchLoadTblStats(tableModel.request); - // Mark the trace loading while its type-specific fetch obtains the - // current table data (and, for grouped traces, its group subset). const changes = getDataChangesForMappings({mappings, traceNum}); + // Filtering reloads table-backed traces. Clear fixed ranges so the chart + // rescales to filtered rows, but keep reversed-axis autorange intact. if (isFilterEvent) { - changes['layout.xaxis.autorange'] = true; + changes['layout.xaxis.autorange'] = getFilterResetAutorange(chartData.layout?.xaxis); changes['layout.xaxis.range'] = undefined; - changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.autorange'] = getFilterResetAutorange(chartData.layout?.yaxis); changes['layout.yaxis.range'] = undefined; } diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index 69e60ba27e..48314fe6a8 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -16,7 +16,7 @@ import {dispatchAddViewerItems, dispatchUpdateCustom, dispatchRemoveViewerItems, import {DEFAULT_PLOT2D_VIEWER_ID} from '../visualize/VisConst'; import {applyDefaults, flattenAnnotations, formatColExpr, getPointIdx, getRowIdx, handleTableSourceConnections, clearChartConn, newTraceFrom, setupTableWatcher, HIGHLIGHTED_PROPS, SELECTED_PROPS, TBL_SRC_PATTERN, - getSelIndexes, getTblIdFromChart, isGroupedChart, combineAllTraceFrom} from './ChartUtil.js'; + getSelIndexes, getTblIdFromChart, isGroupedChart, combineAllTraceFrom, getFilterResetAutorange} from './ChartUtil.js'; import {FilterInfo} from '../tables/FilterInfo.js'; import {SelectInfo} from '../tables/SelectInfo.js'; import {REINIT_APP} from '../core/CoreConst'; @@ -478,11 +478,12 @@ function chartFilterSelection(action) { const newRequest = Object.assign({}, request, {filters: filterInfoCls.serialize(formatKey)}); TablesCntlr.dispatchTableFilter(newRequest); + const {layout} = getChartData(chartId) ?? {}; dispatchChartUpdate({chartId, changes:{ selection: undefined, - 'layout.xaxis.autorange': true, + 'layout.xaxis.autorange': getFilterResetAutorange(layout?.xaxis), 'layout.xaxis.range': undefined, - 'layout.yaxis.autorange': true, + 'layout.yaxis.autorange': getFilterResetAutorange(layout?.yaxis), 'layout.yaxis.range': undefined }}); } diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index 6c00df176e..03478a6449 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -6,7 +6,7 @@ import {getTblById, getColumns, getColumn, doFetchTable, stripColumnNameQuotes} import {cloneRequest, makeSubQueryRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {getFiltersAsSql} from '../../tables/FilterInfo.js'; import {dispatchChartUpdate, dispatchError, getChartData, getTraceSymbol, hasUpperLimits, hasLowerLimits} from '../ChartsCntlr.js'; -import {formatColExpr, getDataChangesForMappings, getOriginalRowIndexes, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; +import {formatColExpr, getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; import {getTraceTSEntries as heatmapTSGetter} from './FireflyHeatmap.js'; import {errorTypeFieldKey} from '../ui/options/Errors.jsx'; @@ -92,9 +92,10 @@ async function fetchData(chartId, traceNum, tablesource) { addOtherChanges({changes, chartId, traceNum, tablesource, tableModel: originalTableModel}); if (resetAxes) { - changes['layout.xaxis.autorange'] = true; + const {layout} = getChartData(chartId) ?? {}; + changes['layout.xaxis.autorange'] = getFilterResetAutorange(layout?.xaxis); changes['layout.xaxis.range'] = undefined; - changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.autorange'] = getFilterResetAutorange(layout?.yaxis); changes['layout.yaxis.range'] = undefined; } diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index d5a211f4e3..b366b07a14 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -6,7 +6,7 @@ import {isEmpty, pickBy, cloneDeep, set, uniqueId} from 'lodash'; import {getTblById, getColumn, doFetchTable, getColumnIdx} from '../../tables/TableUtil.js'; import {getSpectrumDM, REF_POS} from '../../voAnalyzer/SpectrumDM.js'; import {dispatchChartUpdate, dispatchError, getChartData} from '../ChartsCntlr.js'; -import {getDataChangesForMappings, getOriginalRowIndexes, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; +import {getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; import {addOtherChanges, createChartTblRequest, getTraceTSEntries as genericTSGetter} from './FireflyGenericData.js'; import {quoteNonAlphanumeric} from '../../util/expr/Variable.js'; @@ -65,9 +65,10 @@ async function fetchData(chartId, traceNum, tablesource) { addOtherChanges({changes, chartId, traceNum, tablesource, tableModel: originalTableModel}); if (resetAxes) { - changes['layout.xaxis.autorange'] = true; + const {layout} = getChartData(chartId) ?? {}; + changes['layout.xaxis.autorange'] = getFilterResetAutorange(layout?.xaxis); changes['layout.xaxis.range'] = undefined; - changes['layout.yaxis.autorange'] = true; + changes['layout.yaxis.autorange'] = getFilterResetAutorange(layout?.yaxis); changes['layout.yaxis.range'] = undefined; } From 4ecbaf373f0fdc49a5dc21bb3023a20ed98bfa9d Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Tue, 11 Aug 2026 00:23:15 -0700 Subject: [PATCH 08/11] overplot bug fix --- src/firefly/js/charts/ChartUtil.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 207531bfab..3e89a36ff3 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -996,9 +996,11 @@ function getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}) { const baseState = fireflyData?.[activeTrace]?.groupBy?.baseState; if (!isEmpty(baseState?.mappings)) return baseState.mappings; - const sameTableBaseState = fireflyData?.find((fd) => - fd?.groupBy?.baseState?.tbl_id === tbl_id && !isEmpty(fd.groupBy.baseState.mappings)); - if (sameTableBaseState) return sameTableBaseState.groupBy.baseState.mappings; + const sameTableBaseState = fireflyData?.find((fd) => { + const baseState = fd?.groupBy?.baseState; + return !isEmpty(baseState?.mappings) && (!tbl_id || baseState.tbl_id === tbl_id); + })?.groupBy?.baseState; + if (sameTableBaseState) return sameTableBaseState.mappings; return {}; } From 93e982df07ce2f26e0d9bc86d04516f38f84ca3d Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Tue, 11 Aug 2026 14:03:55 -0700 Subject: [PATCH 09/11] cleanup --- src/firefly/js/charts/ChartUtil.js | 6 +++--- src/firefly/js/charts/ChartsCntlr.js | 2 +- src/firefly/js/charts/TableStatsCntlr.js | 9 +++++---- src/firefly/js/charts/dataTypes/FireflyGenericData.js | 4 ++-- src/firefly/js/charts/dataTypes/FireflySpectrum.js | 2 +- src/firefly/js/charts/ui/ChartSelectPanel.jsx | 3 +-- src/firefly/js/charts/ui/PlotlyChartArea.jsx | 2 +- src/firefly/js/charts/ui/PlotlyWrapper.jsx | 4 ++-- src/firefly/js/charts/ui/options/SpectralLines.jsx | 3 ++- src/firefly/js/charts/ui/options/SpectrumOptions.jsx | 6 ++---- 10 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 3e89a36ff3..5c9835cafa 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -756,8 +756,8 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac const tablesources = makeTableSources(chartId, data, fireflyData); const {tablesources:oldTablesources=[], activeTrace, curveNumberMap=[]} = getChartData(chartId); - // Partial data updates contain arrays, not table-source metadata. Keep the - // existing watchers for those updates; only reconnect on source changes. + // Some chart updates only modify Plotly trace/layout state, not the table source + // Preserve existing table watchers unless makeTableSources found a real source update const hasTableSourceUpdates = Array.isArray(tablesources) && tablesources.some((ts) => ts?.tbl_id && (!isEmpty(ts.mappings) || typeof ts.fetchData === 'function')); if (!hasTableSourceUpdates) { @@ -915,7 +915,7 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { const changes = getDataChangesForMappings({mappings, traceNum}); // Filtering reloads table-backed traces. Clear fixed ranges so the chart - // rescales to filtered rows, but keep reversed-axis autorange intact. + // rescales to filtered rows, but keep reversed-axis autorange intact if (isFilterEvent) { changes['layout.xaxis.autorange'] = getFilterResetAutorange(chartData.layout?.xaxis); changes['layout.xaxis.range'] = undefined; diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index 48314fe6a8..a2cf32abe9 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -371,7 +371,7 @@ function chartHighlight(action) { const hlTrace = newTraceFrom(data[traceNum], [highlighted], HIGHLIGHTED_PROPS, traceAnnotations); dispatchChartUpdate({chartId, changes: {highlighted: hlTrace}}); } else if (data[traceNum]) { - // Update the visual highlight immediately; the table watcher may be rebuilding. + //update the visual highlight immediately const traceAnnotations = get(fireflyData, `${traceNum}.annotations`); const hlTrace = newTraceFrom(data[traceNum], [highlighted], HIGHLIGHTED_PROPS, traceAnnotations); dispatchChartUpdate({chartId, changes: {highlighted: hlTrace}}); diff --git a/src/firefly/js/charts/TableStatsCntlr.js b/src/firefly/js/charts/TableStatsCntlr.js index ead5ed3a36..0444a72fb3 100644 --- a/src/firefly/js/charts/TableStatsCntlr.js +++ b/src/firefly/js/charts/TableStatsCntlr.js @@ -27,7 +27,8 @@ export const TBLSTATS_DATA_KEY = 'tblstats'; export const LOAD_TBL_STATS = `${TBLSTATS_DATA_KEY}/LOAD_TBL_STATS`; export const UPDATE_TBL_STATS = `${TBLSTATS_DATA_KEY}/UPDATE_TBL_STATS`; -const statsInFlight = new Set(); +//Avoid duplicate column-stat requests while the first request for a table is still pending +const pendingStatsTblIds = new Set(); export default {actionCreators, reducers}; @@ -60,8 +61,8 @@ export function dispatchLoadTblStats(searchRequest, dispatcher= flux.process) { if (resultSetID !== resultSetIDNow || curNumericColCnt !== numericColCnt) { // also reload stats if the number of numeric columns changes. - if (statsInFlight.has(tbl_id)) return; - statsInFlight.add(tbl_id); + if (pendingStatsTblIds.has(tbl_id)) return; + pendingStatsTblIds.add(tbl_id); dispatcher({type: LOAD_TBL_STATS, payload: {searchRequest}}); } } @@ -201,7 +202,7 @@ function fetchTblStats(dispatch, activeTableServerRequest) { })); } ).finally(() => { - statsInFlight.delete(tbl_id); + pendingStatsTblIds.delete(tbl_id); const currentRequest = getTblById(tbl_id)?.request; const currentResultSetID = get(getTblById(tbl_id), 'tableMeta.resultSetID'); if (currentRequest && currentResultSetID !== resultSetIDOnSubmit) { diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index 03478a6449..72f85a12c9 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -78,7 +78,7 @@ async function fetchData(chartId, traceNum, tablesource) { dispatchError(chartId, traceNum, reason); }); - // Ignore a fetch only when both source generations are known and differ. + //ignore stale results from an older table source if this trace was reconnected while fetching const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; if (tableModel?.error) { @@ -139,7 +139,7 @@ export function createChartTblRequest(chartId, traceNum, tablesource) { if (groupFilter) { const inclCols = (sreq.inclCols ? sreq.inclCols + ',' : '') + '"ROW_NUM" as "ORIG_IDX"'; const tableFilter = getFiltersAsSql(tbl_id); - // Keep selection/table filters when adding the generated trace's group predicate. + //apply the table filter and this trace's group filter together const filters = tableFilter ? `(${tableFilter}) AND (${groupFilter})` : groupFilter; sreq = makeSubQueryRequest(request, sreq.title, sreq.params,{filters, inclCols, pageSize: MAX_ROW}); } diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index b366b07a14..d50f7efe53 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -51,7 +51,7 @@ async function fetchData(chartId, traceNum, tablesource) { dispatchError(chartId, traceNum, reason); }); - // Ignore a fetch only when both source generations are known and differ. + //ignore stale results from an older table source if this trace was reconnected while fetching const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; if (tableModel?.error) { diff --git a/src/firefly/js/charts/ui/ChartSelectPanel.jsx b/src/firefly/js/charts/ui/ChartSelectPanel.jsx index b74da246c6..f9f3c00639 100644 --- a/src/firefly/js/charts/ui/ChartSelectPanel.jsx +++ b/src/firefly/js/charts/ui/ChartSelectPanel.jsx @@ -297,8 +297,7 @@ SyncedOptionsUI.propTypes = { */ export function showChartsDialog(chartId,chartAction, tbl_id) { const {data, fireflyData, activeTrace, groupId} = getChartData(chartId); - const workingTblId = tbl_id ?? (get(data, `${activeTrace}.tbl_id`) || - get(fireflyData, `${activeTrace}.tbl_id`) || groupId); + const workingTblId = tbl_id ?? (data?.[activeTrace]?.tbl_id || fireflyData?.[activeTrace]?.tbl_id || groupId); const popupId ='chartOptionsDialog'; const dialogContent= ( diff --git a/src/firefly/js/charts/ui/PlotlyChartArea.jsx b/src/firefly/js/charts/ui/PlotlyChartArea.jsx index d7e00466e9..b45cc73cf7 100644 --- a/src/firefly/js/charts/ui/PlotlyChartArea.jsx +++ b/src/firefly/js/charts/ui/PlotlyChartArea.jsx @@ -77,7 +77,7 @@ export function PlotlyChartArea({chartId, widthPx, heightPx, thumbnail}) { const {chartWidth, chartHeight} = calculateChartSize(widthPx, heightPx, xyratio, stretch); - const showlegend = isGroupedChart(chartId) ? true : get(layout, 'showlegend', data.length > 1); + const showlegend = isGroupedChart(chartId) ? true : (layout?.showlegend ?? data.length > 1); const playout = cloneDeep({showlegend, ...adjustLayout(layout, theme), width: chartWidth, height: chartHeight, annotations, ...hoverLayout}); const style = {float: 'left'}; diff --git a/src/firefly/js/charts/ui/PlotlyWrapper.jsx b/src/firefly/js/charts/ui/PlotlyWrapper.jsx index e9f66cf71b..3c86be1117 100644 --- a/src/firefly/js/charts/ui/PlotlyWrapper.jsx +++ b/src/firefly/js/charts/ui/PlotlyWrapper.jsx @@ -344,8 +344,8 @@ export class PlotlyWrapper extends Component { const {layout} = getChartData(chartId); if (layout && !this.props.thumbnail) { Object.entries(changes).forEach( ([k, v]) => { - // Plotly emits private selection edit paths such as selections[0].yref. - // Selection state is persisted explicitly by PlotlyChartArea. + //ignore Plotly's transient selection-box layout edits, e.g. selections[0].yref. + //the supported selection state is captured from plotly_selected in PlotlyChartArea if (k.startsWith('selections[')) return; if (k === 'xaxis' && Array.isArray(v)) { set(layout, 'xaxis.range', v); diff --git a/src/firefly/js/charts/ui/options/SpectralLines.jsx b/src/firefly/js/charts/ui/options/SpectralLines.jsx index 87c57052fa..d2cf1df551 100644 --- a/src/firefly/js/charts/ui/options/SpectralLines.jsx +++ b/src/firefly/js/charts/ui/options/SpectralLines.jsx @@ -130,7 +130,8 @@ export function SpectralLinesOptions({activeTrace, chartId}) { isKnownRefPos(getChartData(chartId)?.fireflyData?.[activeTrace]?.spectralFrame?.refPos), [chartId, activeTrace]); - // read the current trace's field-group value, fall back to persisted state while it mounts + // The field can be undefined for one render while the dialog mounts + // Use live form state when present, otherwise fall back to chart state const isEnabledField = useStoreConnector(() => getFieldVal(groupKey, ENABLED_KEY), [groupKey]); const isEnabled = hasSpectralFrame && (isEnabledField ?? initialEnabled); diff --git a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx index 569dee76eb..ab5c0c797d 100644 --- a/src/firefly/js/charts/ui/options/SpectrumOptions.jsx +++ b/src/firefly/js/charts/ui/options/SpectrumOptions.jsx @@ -260,7 +260,7 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren const {data, fireflyData} = getChartData(chartId); const {spectralAxis={}, fluxAxis={}} = getSpectrumDM(getTblById(tbl_id)) || {}; // A grouped spectrum's generated traces share the same axes. Preserve each trace's mappings so - // changing one trace's style cannot leave the next selected trace with a partial table source. + // changing one trace's style cannot leave the next selected trace with a partial table source const tracesToPreserve = isGroupedChart(chartId) ? range(data.length) : [activeTrace]; tracesToPreserve.forEach((traceNum) => { const {mappings={}} = getChartProps(chartId, tbl_id, traceNum); @@ -292,9 +292,7 @@ export function submitChangesSpectrum({chartId, activeTrace, fields, tbl_id, ren const yUnitChanged = yUnit !== undefined && yUnit !== fireflyData?.[activeTrace]?.yUnit; const spectralFrameChanged = !isEqual(sfOptionFields, currentSFOptionFields); - //Only propagate axis settings when an axis setting actually changed. In particular, changing - //a grouped trace's style or symbol must not cause missing spectralFrameOption metadata on a - //sibling trace to trigger a unit conversion and rewrite lambda/Flux expressions. + // when units or spectral frame options change, apply it to the other/inactive traces as well range(data.length).forEach((idx) => { if (idx !== activeTrace) { // get units and spectral frame options from the chart data of inactive trace From 5b1db6bc87885dbd44f747704afca2b07dff69bb Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Fri, 14 Aug 2026 17:51:20 -0700 Subject: [PATCH 10/11] several robust fixes --- src/firefly/js/charts/ChartUtil.js | 73 ++++++++++++++++--- src/firefly/js/charts/ChartsCntlr.js | 67 +++++++++++++++-- .../js/charts/dataTypes/FireflyGenericData.js | 9 +-- .../js/charts/dataTypes/FireflyHeatmap.js | 45 ++++++++---- .../js/charts/dataTypes/FireflyHistogram.js | 36 +++++++-- .../js/charts/dataTypes/FireflySpectrum.js | 7 +- src/firefly/js/charts/ui/PlotlyChartArea.jsx | 8 +- src/firefly/js/charts/ui/PlotlyToolbar.jsx | 14 +++- src/firefly/js/charts/ui/PlotlyWrapper.jsx | 4 +- .../ui/options/FireflyHistogramOptions.jsx | 12 ++- .../js/charts/ui/options/HeatmapOptions.jsx | 28 ++++++- 11 files changed, 244 insertions(+), 59 deletions(-) diff --git a/src/firefly/js/charts/ChartUtil.js b/src/firefly/js/charts/ChartUtil.js index 5c9835cafa..f4dbf444b3 100644 --- a/src/firefly/js/charts/ChartUtil.js +++ b/src/firefly/js/charts/ChartUtil.js @@ -91,6 +91,23 @@ export function getOriginalRowIndexes(tableModel) { return origIdx >= 0 ? tableModel?.tableData?.data?.map((row) => row[origIdx]) : undefined; } +/** + * Check whether an async result belongs to the source currently installed + * for a chart trace. Cancelled requests may still finish, so stale results + * must be ignored after a source is removed or replaced. + * + * @param {string} chartId chart identifier + * @param {number} traceNum trace index + * @param {Object} tablesource source captured by the request + * @returns {boolean} true when the source is still current + */ +export function isCurrentTableSource(chartId, traceNum, tablesource) { + const chartData = getChartData(chartId); + const currentSource = chartData?.tablesources?.[traceNum]; + return Boolean(chartData?.mounted && currentSource?._sourceId && tablesource?._sourceId && + currentSource._sourceId === tablesource._sourceId); +} + /** * Does the application only support single trace * @returns {*} true, if all charts are single trace @@ -763,6 +780,7 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac if (!hasTableSourceUpdates) { oldTablesources.forEach((traceTS, idx) => { if (!traceTS?.tbl_id) return; + if (!traceTS._sourceId) traceTS._sourceId = uniqueId('chart-source-'); if (!traceTS._cancel) traceTS._cancel = setupTableWatcher(chartId, traceTS, idx); if (syncExistingSources) updateChartData(chartId, idx, traceTS); }); @@ -815,6 +833,7 @@ export function handleTableSourceConnections({chartId, data, fireflyData, replac // fetch data syncs highlighted and selected with the table updateChartData(chartId, idx, traceTS); } else { + if (!traceTS._sourceId) traceTS._sourceId = uniqueId('chart-source-'); if (idx === activeTrace && isFullyLoaded(traceTS.tbl_id)) { // update highlighted and selected const tableModel = getTblById(traceTS.tbl_id); @@ -880,11 +899,11 @@ function updateChartData(chartId, traceNum, tablesource, action={}) { // Table filters are global, so handle them with the current source, // otherwise a stale source may point to an old grouped trace or old mappings const currentSource = chartData.tablesources?.[traceNum]; - const sourceWasReplaced = action.type && tablesource._sourceId && currentSource?._sourceId && - tablesource._sourceId !== currentSource._sourceId; + const sourceIsCurrent = isCurrentTableSource(chartId, traceNum, tablesource); + const sourceWasReplaced = action.type && !sourceIsCurrent; const isFilterEvent = action.type === TABLE_LOADED && action.payload.invokedBy === TABLE_FILTER; if (sourceWasReplaced) { - if (!isFilterEvent) return; + if (!isFilterEvent || !currentSource) return; tablesource = currentSource; } @@ -968,7 +987,9 @@ function makeTableSources(chartId, data=[], fireflyData=[]) { // set up table server request parameters (options) for firefly specific charts const chartDataType = get(fireflyData[traceNum], 'dataType'); if (!isEmpty(ds)) { - Object.assign(ds, getTraceTSEntries({chartDataType, traceTS: ds, chartId, traceNum})); + const traceOptions = get(fireflyData, [traceNum, 'options']); + Object.assign(ds, getTraceTSEntries({chartDataType, + traceTS: {...ds, options: traceOptions}, chartId, traceNum})); } return ds; }); @@ -1005,6 +1026,43 @@ function getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}) { return {}; } +function getTypeSpecificTableSourceMappings({fireflyData, activeTrace}) { + const dataType = fireflyData?.[activeTrace]?.dataType; + const options = fireflyData?.[activeTrace]?.options; + + if (dataType === 'fireflyHeatmap' && options) { + return { + x: options.xColOrExpr, + y: options.yColOrExpr + }; + } + + return {}; +} + +/** + * Resolve table mappings in priority order: + * 1. Current generic source mappings. + * 2. Grouped-chart base mappings. + * 3. Type-specific mappings, currently heatmap X/Y options. + * + * Grouped charts and heatmaps store mappings outside the normal source + * structure, so they require explicit fallbacks. + * + * @returns {Object} resolved table-column mappings + */ +function getResolvedTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}) { + const genericMappings = getGenericTableSourceMappings({ + data, fireflyData, tablesources, activeTrace, tbl_id + }); + if (!isEmpty(genericMappings)) return genericMappings; + + const groupedMappings = getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}); + if (!isEmpty(groupedMappings)) return groupedMappings; + + return getTypeSpecificTableSourceMappings({fireflyData, activeTrace}); +} + function getTraceTSEntries({chartDataType, traceTS, chartId, traceNum}) { if (chartDataType === 'fireflyHistogram') { return histogramTSGetter({traceTS, chartId, traceNum}); @@ -1500,16 +1558,13 @@ export function getChartProps(chartId, tbl_id, activeTrace) { activeTrace = activeTrace ?? rest.activeTrace ?? 0; const activeTablesource = get(tablesources, [activeTrace]); tbl_id = tbl_id || activeTablesource?.tbl_id || data?.[activeTrace]?.tbl_id || fireflyData?.[activeTrace]?.tbl_id; - const genericMappings = getGenericTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}); - const mappings = !isEmpty(genericMappings) - ? genericMappings - : getGroupedTableSourceMappings({fireflyData, activeTrace, tbl_id}); + const dataType = fireflyData?.[activeTrace]?.dataType; + const mappings = getResolvedTableSourceMappings({data, fireflyData, tablesources, activeTrace, tbl_id}); const tablesource = (activeTablesource || tbl_id) ? {...(activeTablesource || {}), tbl_id, mappings} : undefined; const multiTrace = activeTrace > 0 || data?.length > 1; const type = get(data, `${activeTrace}.type`, 'scatter'); - const dataType = fireflyData?.[activeTrace]?.dataType; const noColor = !hasMarkerColor(type); const noXY = hasNoXY(type, tablesource); const isXNotNumeric = noXY ? undefined : isNonNumColumn(tbl_id, mappings?.x); diff --git a/src/firefly/js/charts/ChartsCntlr.js b/src/firefly/js/charts/ChartsCntlr.js index a2cf32abe9..c1d780436c 100644 --- a/src/firefly/js/charts/ChartsCntlr.js +++ b/src/firefly/js/charts/ChartsCntlr.js @@ -2,20 +2,20 @@ * License information at https://github.com/Caltech-IPAC/firefly/blob/master/License.txt */ -import {cloneDeep, has, get, isArray, isEmpty, isString, isUndefined, omit, omitBy, set, range, unset} from 'lodash'; +import {cloneDeep, has, get, isArray, isEmpty, isString, isUndefined, omit, omitBy, set, range, unset, uniqueId} from 'lodash'; import shallowequal from 'shallowequal'; import {flux} from '../core/ReduxFlux.js'; import {updateSet, updateObject, toBoolean} from '../util/WebUtil.js'; import {Logger} from '../util/Logger.js'; -import {getTblById, getColumns, isFullyLoaded, COL_TYPE} from '../tables/TableUtil.js'; +import {getTblById, getColumn, getColumns, isFullyLoaded, COL_TYPE} from '../tables/TableUtil.js'; import {dispatchAddActionWatcher} from '../core/MasterSaga.js'; import * as TablesCntlr from '../tables/TablesCntlr.js'; import {dispatchAddViewerItems, dispatchUpdateCustom, dispatchRemoveViewerItems, getMultiViewRoot, getViewer} from '../visualize/MultiViewCntlr.js'; import {DEFAULT_PLOT2D_VIEWER_ID} from '../visualize/VisConst'; import {applyDefaults, flattenAnnotations, formatColExpr, getPointIdx, getRowIdx, handleTableSourceConnections, - clearChartConn, newTraceFrom, setupTableWatcher, HIGHLIGHTED_PROPS, SELECTED_PROPS, TBL_SRC_PATTERN, + clearChartConn, newTraceFrom, setupTableWatcher, updateHighlighted, updateSelection, HIGHLIGHTED_PROPS, SELECTED_PROPS, TBL_SRC_PATTERN, getSelIndexes, getTblIdFromChart, isGroupedChart, combineAllTraceFrom, getFilterResetAutorange} from './ChartUtil.js'; import {FilterInfo} from '../tables/FilterInfo.js'; import {SelectInfo} from '../tables/SelectInfo.js'; @@ -313,13 +313,46 @@ function chartTraceRemove(action) { tablesources.forEach((ts,i) => { if (i>=traceNum && ts && ts._cancel) { ts._cancel(); - if (i !== traceNum) { - const newIdx = i-1; - ts._cancel = setupTableWatcher(chartId, ts, newIdx); - } + // mark the cancelled watcher as inactive so it is recreated with the + // correct trace index after the trace arrays are reduced. + ts._cancel = undefined; } }); dispatch(action); + + // Reconnect surviving table sources using their new trace indices; + // preserve their metadata, and assign new IDs to sources that moved + const {tablesources:remainingSources=[], mounted} = getChartData(chartId) || {}; + if (mounted > 0 && remainingSources.length > 0) { + const reconnectedSources = remainingSources.map((tablesource, idx) => { + if (!tablesource?.tbl_id) return tablesource; + //give moved sources a new ID so requests from their old index are ignored. + const reconnectedSource = idx >= traceNum + ? {...tablesource, _sourceId: uniqueId('chart-source-')} + : tablesource; + return {...reconnectedSource, _cancel: setupTableWatcher(chartId, reconnectedSource, idx)}; + }); + dispatchChartUpdate({chartId, changes: {tablesources: reconnectedSources}}); + + const activeSource = reconnectedSources[getChartData(chartId)?.activeTrace]; + const activeTable = activeSource?.tbl_id && getTblById(activeSource.tbl_id); + if (activeSource && activeTable) { + const currentChart = getChartData(chartId); + const activeTraceNow = currentChart.activeTrace; + const axisChanges = {}; + const xMapping = activeSource.mappings?.x; + const currentXTitle = currentChart.layout?.xaxis?.title?.text; + //restore the shared X-axis label if trace removal left it empty (historgam removal did this, even on ops) + if (xMapping && (!currentXTitle || currentXTitle === 'undefined')) { + const xColumn = getColumn(activeTable, xMapping); + const xUnit = get(xColumn, 'units', ''); + axisChanges['layout.xaxis.title.text'] = xMapping + (xUnit ? ` (${xUnit})` : ''); + } + if (!isEmpty(axisChanges)) dispatchChartUpdate({chartId, changes: axisChanges}); + updateHighlighted(chartId, activeTraceNow, activeTable.highlightedRow); + updateSelection(chartId, activeTable.selectInfo || {}); + } + } }; } @@ -436,7 +469,14 @@ function chartSelect(action) { function chartFilterSelection(action) { return (dispatch) => { const {chartId} = action.payload; - const {activeTrace=0, selection} = getChartData(chartId) || {}; + const {activeTrace=0, selection, data=[], fireflyData=[]} = getChartData(chartId) || {}; + const activeTraceType = data?.[activeTrace]?.type; + const activeDataType = fireflyData?.[activeTrace]?.dataType; + + //Histograms use bins and counts rather than table-backed X/Y points,so chart filtering does not apply + //ignore the action even if a transient selection state or toolbar entry is still present while the trace is loading + if (activeDataType === 'fireflyHistogram' || activeTraceType === 'bar') return; + const tbl_id = getTblIdFromChart(chartId, activeTrace); const tableModel = tbl_id && getTblById(tbl_id); const {mappings={}} = tableModel ? getChartProps(chartId, tbl_id, activeTrace) : {}; @@ -883,6 +923,17 @@ function removeTrace({chartId, traceNum}) { } } + //Ensure trace-removal state has a valid active trace and curve map when + //an overplot was created before its curve map was initialized + //(this is a defensive fix for bugs encountred encountered involving partially-created / asynchronously changing overplots) + if (!changes.curveNumberMap && changes.data?.length) { + const newActiveTrace = Math.min(activeTrace ?? 0, changes.data.length - 1); + changes.activeTrace = newActiveTrace; + changes.curveNumberMap = range(changes.data.length) + .filter((idx) => idx !== newActiveTrace) + .concat(newActiveTrace); + } + return {changes, moreChanges}; } diff --git a/src/firefly/js/charts/dataTypes/FireflyGenericData.js b/src/firefly/js/charts/dataTypes/FireflyGenericData.js index 72f85a12c9..afe94a6962 100644 --- a/src/firefly/js/charts/dataTypes/FireflyGenericData.js +++ b/src/firefly/js/charts/dataTypes/FireflyGenericData.js @@ -6,7 +6,7 @@ import {getTblById, getColumns, getColumn, doFetchTable, stripColumnNameQuotes} import {cloneRequest, makeSubQueryRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {getFiltersAsSql} from '../../tables/FilterInfo.js'; import {dispatchChartUpdate, dispatchError, getChartData, getTraceSymbol, hasUpperLimits, hasLowerLimits} from '../ChartsCntlr.js'; -import {formatColExpr, getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; +import {formatColExpr, getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, isCurrentTableSource, updateHighlighted, updateSelection, isScatter2d, getMaxScatterRows, getMinScatterGLRows} from '../ChartUtil.js'; import {getTraceTSEntries as heatmapTSGetter} from './FireflyHeatmap.js'; import {errorTypeFieldKey} from '../ui/options/Errors.jsx'; @@ -75,12 +75,11 @@ async function fetchData(chartId, traceNum, tablesource) { const sreq = createChartTblRequest(chartId, traceNum, tablesource); const tableModel = await doFetchTable(sreq).catch((reason) => { - dispatchError(chartId, traceNum, reason); + if (isCurrentTableSource(chartId, traceNum, tablesource)) dispatchError(chartId, traceNum, reason); }); //ignore stale results from an older table source if this trace was reconnected while fetching - const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; - if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; + if (!isCurrentTableSource(chartId, traceNum, tablesource)) return; if (tableModel?.error) { return dispatchError(chartId, traceNum, tableModel.error); } @@ -231,7 +230,7 @@ export function addScatterChanges({changes, chartId, traceNum, tablesource, tabl const colors = get(changes, [`data.${traceNum}.marker.color`]); let cTipLabel = isArray(colors) ? get(mappings, 'marker.color') : ''; - if (cTipLabel.length > 20) { + if (cTipLabel?.length > 20) { cTipLabel = 'c'; } diff --git a/src/firefly/js/charts/dataTypes/FireflyHeatmap.js b/src/firefly/js/charts/dataTypes/FireflyHeatmap.js index f541361e2d..b174157fa3 100644 --- a/src/firefly/js/charts/dataTypes/FireflyHeatmap.js +++ b/src/firefly/js/charts/dataTypes/FireflyHeatmap.js @@ -5,7 +5,7 @@ import {get, isArray} from 'lodash'; import {getTblById, getColumn, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; import {makeTableFunctionRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {dispatchChartUpdate, dispatchError, getChartData} from '../ChartsCntlr.js'; -import {isScatter2d, getMaxScatterRows, singleTraceUI, handleBigInt} from '../ChartUtil.js'; +import {isCurrentTableSource, isScatter2d, getMaxScatterRows, singleTraceUI, handleBigInt} from '../ChartUtil.js'; import {serializeDecimateInfo, parseDecimateKey} from '../../tables/Decimate.js'; import BrowserInfo from '../../util/BrowserInfo.js'; import {formatColExpr} from '../ChartUtil.js'; @@ -26,9 +26,13 @@ const DEFBINS = 100; * @param p.traceNum */ export function getTraceTSEntries({traceTS, chartId, traceNum}) { - const {mappings} = traceTS; + const {mappings={}, options:traceOptions={}} = traceTS; - if (!mappings) return {}; + //During partial chart updates, trace removal, pinning, filtering, or fullscreen transitions, + //the table source could temporarily contain empty mappings, which is why we need this fallback to traceOptions + const xColOrExpr = mappings.x || traceOptions.xColOrExpr; + const yColOrExpr = mappings.y || traceOptions.yColOrExpr; + if (!xColOrExpr || !yColOrExpr) return {}; const {fireflyData, fireflyLayout} = getChartData(chartId) || {}; // server call parameters @@ -47,9 +51,7 @@ export function getTraceTSEntries({traceTS, chartId, traceNum}) { const ymax = get(fireflyLayout, 'yaxis.max'); const options = { - xColOrExpr: get(mappings, 'x'), - yColOrExpr: get(mappings, 'y'), - maxbins, xyratio, xmin, xmax, ymin, ymax + xColOrExpr, yColOrExpr, maxbins, xyratio, xmin, xmax, ymin, ymax }; return {options, fetchData}; @@ -75,7 +77,9 @@ function fetchData(chartId, traceNum, tablesource) { const numericCols = getColumns(tableModel, COL_TYPE.NUMBER).map((c) => c.name); const {request} = tableModel; - const {xColOrExpr, yColOrExpr, maxbins, xyratio, xmin, xmax, ymin, ymax} = options; + const {xColOrExpr, yColOrExpr, maxbins, xyratio, xmin, xmax, ymin, ymax} = options || {}; + //A stale/reindexed source can briefly lack axis expressions, do not attempt to fetch in that case + if (!xColOrExpr || !yColOrExpr) return; const xColName = numericCols.includes(xColOrExpr) ? xColOrExpr : 'xColumnExpression'; const asX = (xColName === xColOrExpr) ? '' : ` as "${xColName}"`; @@ -94,6 +98,9 @@ function fetchData(chartId, traceNum, tablesource) { {decimate: serializeDecimateInfo(xColName, yColName, maxbins, xyratio, xmin, xmax, ymin, ymax, 0), pageSize: MAX_ROW}); doFetchTable(req).then((tableModel) => { + if (!isCurrentTableSource(chartId, traceNum, tablesource)) { + return; + } if (tableModel.error) { dispatchError(chartId, traceNum, tableModel.error); return; @@ -111,7 +118,7 @@ function fetchData(chartId, traceNum, tablesource) { } }).catch( (reason) => { - dispatchError(chartId, traceNum, reason); + if (isCurrentTableSource(chartId, traceNum, tablesource)) dispatchError(chartId, traceNum, reason); } ); } @@ -127,9 +134,9 @@ function getChanges({tableModel, tablesource, chartId, traceNum}) { return {}; } - // default axes labels for the first trace (remove surrounding quotes, if any) - const xLabel = stripColumnNameQuotes(get(mappings, 'x')); - const yLabel = stripColumnNameQuotes(get(mappings, 'y')); + //prevent missing source mappings from breaking tooltip and axis-label formatting. + const xLabel = stripColumnNameQuotes(mappings?.x || 'x'); + const yLabel = stripColumnNameQuotes(mappings?.y || 'y'); const xTipLabel = xLabel.length > 20 ? xLabel.substring(0,18)+'...' : xLabel; const yTipLabel = yLabel.length > 20 ? yLabel.substring(0,18)+'...' : yLabel; @@ -243,7 +250,11 @@ function getChanges({tableModel, tablesource, chartId, traceNum}) { if (singleTraceUI() || (data?.length===1)) { changes[`data.${traceNum}.colorbar.title.text`] = 'pts'; } else { - changes[`data.${traceNum}.colorbar.title.text`] = get(data, `${traceNum}.name`, 'pts'); + const traceName = get(data, `${traceNum}.name`); + //automatically generated names (for example, "trace 1") are + //implementation details and are not useful colorbar titles + changes[`data.${traceNum}.colorbar.title.text`] = + traceName && !/^trace\s+\d+$/i.test(traceName) ? traceName : 'pts'; } } @@ -304,6 +315,14 @@ export function adjustColorbars({data, fireflyData, layout}) { if (data) { const changes = {}; const nbars = data.filter((d) => get(d, 'colorbar') && get(d, 'showscale', true)).length; + const fireflyColorbarCount = data.filter((d, i) => + get(fireflyData, `${i}.fireflyColorbar`) && get(d, 'colorbar') && get(d, 'showscale', true)).length; + if (fireflyColorbarCount === 0 && nbars === 0 && layout?.legend?.orientation === 'h') { + //firefly colorbars use a horizontal legend while present. Clear + //that temporary override after the last colorbar is removed so + //plotly can restore its normal right-side legend placement + changes['layout.legend.orientation'] = undefined; + } const yside = get(layout, 'yaxis.side'); const yOpposite = (yside === 'right'); let cnt = 1; @@ -337,4 +356,4 @@ export function addColorbarChanges(changes, yOpposite, traceNum, x=1.02) { changes[`data.${traceNum}.colorbar.xanchor`] = 'left'; changes[`data.${traceNum}.colorbar.x`] = x>0 ? x : 1-x; } -} \ No newline at end of file +} diff --git a/src/firefly/js/charts/dataTypes/FireflyHistogram.js b/src/firefly/js/charts/dataTypes/FireflyHistogram.js index 23f5d97567..3802c20d0b 100644 --- a/src/firefly/js/charts/dataTypes/FireflyHistogram.js +++ b/src/firefly/js/charts/dataTypes/FireflyHistogram.js @@ -6,7 +6,7 @@ import {logger} from '../../util/Logger.js'; import {COL_TYPE, getColumn, getColumns, getTblById, doFetchTable, stripColumnNameQuotes} from '../../tables/TableUtil.js'; import {cloneRequest, makeTableFunctionRequest, MAX_ROW} from '../../tables/TableRequestUtil.js'; import {dispatchChartUpdate, dispatchError, getChartData} from '../ChartsCntlr.js'; -import {formatColExpr, handleBigInt} from '../ChartUtil.js'; +import {formatColExpr, handleBigInt, isCurrentTableSource} from '../ChartUtil.js'; import {toMaxFixed, getDecimalPlaces} from '../../util/MathUtil.js'; @@ -22,15 +22,20 @@ import Color from '../../util/Color.js'; * @param p.traceNum * @returns {{options: {}, fetchData: fetchData}} */ -export function getTraceTSEntries({chartId, traceNum}) { +export function getTraceTSEntries({chartId, traceNum, traceTS={}}) { const options = {}; // server call parameters const {fireflyData, layout} = getChartData(chartId) || {}; - const histogramParams = get(fireflyData, `${traceNum}.options`); + //preserve histogram options when chart state updates are partial + const histogramParams = { + ...(traceTS.options || {}), + ...(fireflyData?.[traceNum]?.options || {}) + }; + if (!histogramParams.columnOrExpr) return {}; options.columnExpression = histogramParams.columnOrExpr; - if (get(layout, 'xaxis.type') === 'log') { + if (layout?.xaxis?.type === 'log') { options.columnExpression = 'lg('+histogramParams.columnOrExpr+')'; } if (histogramParams.fixedBinSizeSelection) { // fixed size bins @@ -58,15 +63,28 @@ function fetchData(chartId, traceNum, tablesource) { const {tbl_id, options} = tablesource; const tableModel = getTblById(tbl_id); + if (!tableModel?.request || !options?.columnExpression) { + if (isCurrentTableSource(chartId, traceNum, tablesource)) { + dispatchError(chartId, traceNum, 'Histogram table source is incomplete'); + } + return; + } const numericCols = getColumns(tableModel, COL_TYPE.NUMBER).map((c) => c.name); const {request} = tableModel; const valueColName = 'columnExpression'; + const formattedColumnExpression = formatColExpr({ + colOrExpr: options.columnExpression, + quoted: true, + colNames: numericCols + }); const sreq = cloneRequest(request, { startIdx: 0, pageSize: MAX_ROW, - inclCols: `${formatColExpr({colOrExpr:options.columnExpression, quoted: true, colNames: numericCols})} as "${valueColName}"`, - sortInfo: `ASC,"${valueColName}"` + inclCols: `${formattedColumnExpression} as "${valueColName}"`, + //HistogramProcessor expects the selected value as "columnExpression", + //but the source query must sort by the original expression + sortInfo: `ASC,${formattedColumnExpression}` }); const sreqTblId = uniqueId(request.tbl_id); sreq.META_INFO.tbl_id = sreqTblId; @@ -80,6 +98,8 @@ function fetchData(chartId, traceNum, tablesource) { doFetchTable(req).then( (tableModel) => { + if (!isCurrentTableSource(chartId, traceNum, tablesource)) return; + if (tableModel.error) { dispatchError(chartId, traceNum, tableModel.error); return; @@ -147,7 +167,9 @@ function fetchData(chartId, traceNum, tablesource) { } ).catch( (reason) => { - dispatchError(chartId, traceNum, reason); + if (isCurrentTableSource(chartId, traceNum, tablesource)) { + dispatchError(chartId, traceNum, reason); + } } ); } diff --git a/src/firefly/js/charts/dataTypes/FireflySpectrum.js b/src/firefly/js/charts/dataTypes/FireflySpectrum.js index d50f7efe53..dc46700c73 100644 --- a/src/firefly/js/charts/dataTypes/FireflySpectrum.js +++ b/src/firefly/js/charts/dataTypes/FireflySpectrum.js @@ -6,7 +6,7 @@ import {isEmpty, pickBy, cloneDeep, set, uniqueId} from 'lodash'; import {getTblById, getColumn, doFetchTable, getColumnIdx} from '../../tables/TableUtil.js'; import {getSpectrumDM, REF_POS} from '../../voAnalyzer/SpectrumDM.js'; import {dispatchChartUpdate, dispatchError, getChartData} from '../ChartsCntlr.js'; -import {getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; +import {getDataChangesForMappings, getFilterResetAutorange, getOriginalRowIndexes, isCurrentTableSource, updateHighlighted, updateSelection, getMinScatterGLRows, isGroupedChart} from '../ChartUtil.js'; import {addOtherChanges, createChartTblRequest, getTraceTSEntries as genericTSGetter} from './FireflyGenericData.js'; import {quoteNonAlphanumeric} from '../../util/expr/Variable.js'; @@ -48,12 +48,11 @@ async function fetchData(chartId, traceNum, tablesource) { // set(sreq, 'inclCols', (sreq.inclCols + ', "ROW_IDX"'); const tableModel = await doFetchTable(sreq).catch((reason) => { - dispatchError(chartId, traceNum, reason); + if (isCurrentTableSource(chartId, traceNum, tablesource)) dispatchError(chartId, traceNum, reason); }); //ignore stale results from an older table source if this trace was reconnected while fetching - const currentSourceId = getChartData(chartId)?.tablesources?.[traceNum]?._sourceId; - if (tablesource._sourceId && currentSourceId && currentSourceId !== tablesource._sourceId) return; + if (!isCurrentTableSource(chartId, traceNum, tablesource)) return; if (tableModel?.error) { return dispatchError(chartId, traceNum, tableModel.error); } diff --git a/src/firefly/js/charts/ui/PlotlyChartArea.jsx b/src/firefly/js/charts/ui/PlotlyChartArea.jsx index b45cc73cf7..11aa474fb6 100644 --- a/src/firefly/js/charts/ui/PlotlyChartArea.jsx +++ b/src/firefly/js/charts/ui/PlotlyChartArea.jsx @@ -22,8 +22,10 @@ export function PlotlyChartArea({chartId, widthPx, heightPx, thumbnail}) { const {data=[], isLoading, highlighted, selected, layout={}, activeTrace=0, xyratio, stretch} = useStoreConnector(() => getChartState(chartId), [chartId]); useEffect(()=> { - const {fireflyData} = getChartData(chartId); - handleTableSourceConnections({chartId, data, fireflyData, syncExistingSources: true}); + const {fireflyData, mounted=0} = getChartData(chartId); + handleTableSourceConnections({ + chartId, data, fireflyData, syncExistingSources: mounted === 0 + }); return () => { if (getChartData(chartId)?.mounted === 0) { clearChartConn({chartId}); @@ -99,7 +101,7 @@ export function PlotlyChartArea({chartId, widthPx, heightPx, thumbnail}) { autoDetectResizing={false} thumbnail={thumbnail} doingResize={doingResize} - key={chartId + thumbnail}/> + key={`${chartId}-${thumbnail}-${data.length}`}/>
); } diff --git a/src/firefly/js/charts/ui/PlotlyToolbar.jsx b/src/firefly/js/charts/ui/PlotlyToolbar.jsx index 7e4cf1dd76..77e68d2e2d 100644 --- a/src/firefly/js/charts/ui/PlotlyToolbar.jsx +++ b/src/firefly/js/charts/ui/PlotlyToolbar.jsx @@ -100,11 +100,21 @@ function isSelectable(tbl_id, chartId, type) { const checkY = typeWithY.includes(type); if (!checkX&&!checkY) return false; // chart type has no selection box in tool bar - const {tablesources} = getChartData(chartId); + const {tablesources, fireflyData} = getChartData(chartId); const strCol = ['str', 's', 'char', 'c']; const tableModel = getTblById(tbl_id); const noSelectionTraceIdx = tablesources?.findIndex((tablesource) => { - const {x, y} = get(tablesource, 'mappings') || {}; + const traceNum = tablesources.indexOf(tablesource); + const mappings = tablesource?.mappings || {}; + const traceOptions = fireflyData?.[traceNum]?.options || {}; + //Heatmap axis expressions live in fireflyData.options rather than + //in the table-source mappings used by ordinary traces. + const x = mappings.x || (fireflyData?.[traceNum]?.dataType === 'fireflyHeatmap' + ? traceOptions.xColOrExpr + : undefined); + const y = mappings.y || (fireflyData?.[traceNum]?.dataType === 'fireflyHeatmap' + ? traceOptions.yColOrExpr + : undefined); const dataExp = [x, y]; const noSelectionIdx = [checkX, checkY].findIndex((checkItem, idx) => { diff --git a/src/firefly/js/charts/ui/PlotlyWrapper.jsx b/src/firefly/js/charts/ui/PlotlyWrapper.jsx index 3c86be1117..d0733a97a1 100644 --- a/src/firefly/js/charts/ui/PlotlyWrapper.jsx +++ b/src/firefly/js/charts/ui/PlotlyWrapper.jsx @@ -166,7 +166,9 @@ export class PlotlyWrapper extends Component { let detectedResize= false; const rec= this.div.getBoundingClientRect(); - if (this.lastWidth!==rec.width || this.lastHeight!==rec.height) { + // ignore transient zero-size measurements that can prevent Plotly from rendering + const hasValidSize = rec.width > 0 && rec.height > 0; + if (hasValidSize && (this.lastWidth!==rec.width || this.lastHeight!==rec.height)) { this.lastWidth= rec.width; this.lastHeight=rec.height; detectedResize= true; diff --git a/src/firefly/js/charts/ui/options/FireflyHistogramOptions.jsx b/src/firefly/js/charts/ui/options/FireflyHistogramOptions.jsx index b305e95bd6..6416cb3bbf 100644 --- a/src/firefly/js/charts/ui/options/FireflyHistogramOptions.jsx +++ b/src/firefly/js/charts/ui/options/FireflyHistogramOptions.jsx @@ -47,15 +47,22 @@ export function FireflyHistogramOptions({activeTrace:pActiveTrace, tbl_id:ptbl_i } export function submitChangesFFHistogram({chartId, activeTrace, fields, tbl_id, renderTreeId}) { - const changes = histogramOptionsToChanges(activeTrace, fields, tbl_id); + const changes = histogramOptionsToChanges(chartId, activeTrace, fields, tbl_id); submitChanges({chartId, fields: changes, tbl_id, renderTreeId}); } -function histogramOptionsToChanges(activeTrace, fields, tbl_id) { +function histogramOptionsToChanges(chartId, activeTrace, fields, tbl_id) { const changes = {}; changes[`fireflyData.${activeTrace}.dataType`] = 'fireflyHistogram'; changes[`fireflyData.${activeTrace}.tbl_id`] = tbl_id; + + //keep the trace and Firefly metadata linked to the same table during overploting + changes[`data.${activeTrace}.tbl_id`] = tbl_id; fields && Object.entries(fields).forEach( ([k,v]) => { + // do not let an empty histogram label erase the shared X-axis label while overplotting + if (k === 'layout.xaxis.title.text' && + (!v || v === 'undefined') && + getChartData(chartId)?.data?.length > 1) return; if (['data', 'layout', 'fireflyLayout', 'activeTrace', '_'].find((s) => k.startsWith(s))) { changes[k] = v; } else { @@ -90,4 +97,3 @@ function toHistogramOptions(chartId, activeTrace=0) { }); return histogramOptions; } - diff --git a/src/firefly/js/charts/ui/options/HeatmapOptions.jsx b/src/firefly/js/charts/ui/options/HeatmapOptions.jsx index 11bca78446..90862fb05e 100644 --- a/src/firefly/js/charts/ui/options/HeatmapOptions.jsx +++ b/src/firefly/js/charts/ui/options/HeatmapOptions.jsx @@ -109,10 +109,13 @@ export function TableSourcesOptions({tablesource={}, activeTrace, groupKey, char // _tables. is prefixed the fieldKey. it will be replaced with 'tables::val' on submitChanges. const tbl_id = get(tablesource, 'tbl_id'); const colValStats = getColValStats(tbl_id); + const heatmapOptions = getChartData(chartId)?.fireflyData?.[activeTrace]?.options ?? {}; const xyProps = (xOrY) => ({fldPath:`_tables.data.${activeTrace}.${xOrY}`, label: `${xOrY.toUpperCase()}:`, name: xOrY.toUpperCase(), nullAllowed: false, colValStats, groupKey, slotProps: {control: {orientation}}, - initValue: tablesource?.mappings?.[xOrY] ?? '' + //recover heatmap axes when table-source mappings are unavailable + initValue: tablesource?.mappings?.[xOrY] ?? + heatmapOptions[xOrY === 'x' ? 'xColOrExpr' : 'yColOrExpr'] ?? '' }); const {setVal} = useContext(FieldGroupCtx); @@ -154,14 +157,31 @@ export function submitChangesHeatmap({chartId, activeTrace, fields, tbl_id, rend const dataType = (!tbl_id) ? 'heatmap' : 'fireflyHeatmap'; const changes = { [`data.${activeTrace}.type`] : 'heatmap', - [`fireflyData.${activeTrace}.dataType`] : dataType + [`fireflyData.${activeTrace}.dataType`] : dataType, + //Keep both trace representations linked to the same table + [`data.${activeTrace}.tbl_id`]: tbl_id, + [`fireflyData.${activeTrace}.tbl_id`]: tbl_id }; Object.assign(changes, fields); + //store heatmap axes in both formats required by table sources and heatmap fetches + ['x', 'y'].forEach((axis) => { + const fieldValue = fields?.[`_tables.data.${activeTrace}.${axis}`]; + + if (fieldValue) { + const expression = String(fieldValue).replace(/^tables::/, ''); + + changes[`data.${activeTrace}.${axis}`] = `tables::${expression}`; + changes[`fireflyData.${activeTrace}.options.${axis === 'x' ? 'xColOrExpr' : 'yColOrExpr'}`] = + expression; + } + }); + // reversescale is boolean - changes[`data.${activeTrace}.reversescale`] = toBoolean(get(fields, `data.${activeTrace}.reversescale`)); + const reverseScale = fields?.[`data.${activeTrace}.reversescale`] ?? + fields?.data?.[activeTrace]?.reversescale; + changes[`data.${activeTrace}.reversescale`] = toBoolean(reverseScale); submitChanges({chartId, fields: changes, tbl_id, renderTreeId}); } - From 6c9ad64884046656ba02e5b04c03ed61b88f0b7c Mon Sep 17 00:00:00 2001 From: Kartik Puri Date: Tue, 18 Aug 2026 18:12:31 -0700 Subject: [PATCH 11/11] fix x-y col line delay opening dialog --- .../js/charts/ui/ColumnOrExpression.jsx | 25 ++++++++++++++++--- .../js/charts/ui/options/ScatterOptions.jsx | 8 ++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/firefly/js/charts/ui/ColumnOrExpression.jsx b/src/firefly/js/charts/ui/ColumnOrExpression.jsx index fc155976ab..58ddb665cf 100644 --- a/src/firefly/js/charts/ui/ColumnOrExpression.jsx +++ b/src/firefly/js/charts/ui/ColumnOrExpression.jsx @@ -13,6 +13,7 @@ import MAGNIFYING_GLASS from 'html/images/icons-2014/magnifyingGlass.png'; import {ToolbarButton} from '../../ui/ToolbarButton.jsx'; import {FieldGroupCtx} from '../../ui/FieldGroup.jsx'; import {AutoCompleteInput} from 'firefly/ui/AutoCompleteInput.jsx'; +import {getColumns, getTblById} from '../../tables/TableUtil.js'; export const EXPRESSION_TTIPS = ` @@ -58,12 +59,29 @@ function getOptions(cols, canBeExpression=true) { } -export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,labelWidth=30,name,tooltip, +/** + * Column list for ColumnFld: colValStats if loaded, else the table's own columns (avoids + * waiting on the stats fetch). + * @param {Array} [colValStats] + * @param {string} [tbl_id] + * @returns {Array|undefined} + */ +function getColsFromStatsOrTable(colValStats, tbl_id) { + if (colValStats) { + return colValStats.map((c) => ({name: c.name, units: c.unit, type: c.type, desc: c.descr})); + } + const tableModel = tbl_id && getTblById(tbl_id); + if (!tableModel) return undefined; + return getColumns(tableModel).map(({name, units, type, desc}) => ({name, units, type, desc})); +} + +export function ColumnOrExpression({colValStats,tbl_id,params,groupKey,fldPath,label,labelWidth=30,name,tooltip, nullAllowed,readOnly,initValue, slotProps, sx}) { - if (!colValStats) return
; + const cols = getColsFromStatsOrTable(colValStats, tbl_id); + if (!cols) return
; return ( {return {name: c.name, units: c.unit, type: c.type, desc: c.descr};})} + cols={cols} fieldKey={fldPath} initValue={initValue || params?.[fldPath]} canBeExpression={true} @@ -74,6 +92,7 @@ export function ColumnOrExpression({colValStats,params,groupKey,fldPath,label,la ColumnOrExpression.propTypes = { colValStats: PropTypes.arrayOf(PropTypes.instanceOf(ColValuesStatistics)), + tbl_id: PropTypes.string, params: PropTypes.object, groupKey: PropTypes.string.isRequired, fldPath: PropTypes.string.isRequired, diff --git a/src/firefly/js/charts/ui/options/ScatterOptions.jsx b/src/firefly/js/charts/ui/options/ScatterOptions.jsx index f5e0c0a232..66f90469a4 100644 --- a/src/firefly/js/charts/ui/options/ScatterOptions.jsx +++ b/src/firefly/js/charts/ui/options/ScatterOptions.jsx @@ -318,6 +318,7 @@ export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char name='X' nullAllowed={false} colValStats={colValStats} + tbl_id={tbl_id} groupKey={groupKey} {...withDefaults(props)}/>), deps), Y: useCallback((props) => (), deps), GroupBy: useCallback((props) => { const groupableCols = chartId && isGroupByAllowed(chartId, activeTrace) ? getGroupableColumns(tbl_id) : []; @@ -348,6 +350,7 @@ export const useScatterInputs = ({activeTrace:pActiveTrace, tbl_id:ptbl_id, char name = 'Lower Limit' nullAllowed = {true} colValStats={colValStats} + tbl_id={tbl_id} groupKey = {groupKey} {...withDefaults(props)}/>), deps), Xmax: useCallback((props) => (), deps), Ymin: useCallback((props) => (), deps), Ymax: useCallback((props) => (), deps), ColorMap: useCallback((props) => (), deps), ColorSize: useCallback((props) => (), deps), ColorScale: useCallback((props) => (