From 6e85c23e9efebec58ab6eecf9df53ec610d1fe11 Mon Sep 17 00:00:00 2001 From: Antony Courtney Date: Tue, 17 Oct 2023 15:21:11 -0700 Subject: [PATCH] move showColumnHistograms into viewState where it belongs. --- packages/tad-app/csv/smalltxt.csv | 5 +++ packages/tad-app/csv/tabtest.csv | 5 +++ packages/tad-app/src/electronRenderMain.tsx | 2 +- packages/tadviewer/src/AppState.ts | 3 -- packages/tadviewer/src/PivotRequester.ts | 18 ++++----- packages/tadviewer/src/ViewParams.ts | 3 ++ packages/tadviewer/src/actions.ts | 36 +++++++++++++---- packages/tadviewer/src/components/AppPane.tsx | 1 + .../src/components/DataSourceSidebar.tsx | 2 + .../tadviewer/src/components/GridPane.tsx | 39 ++++++++++--------- .../tadviewer/src/components/PivotSidebar.tsx | 12 ++++++ .../src/components/TadViewerPane.tsx | 8 +++- 12 files changed, 92 insertions(+), 42 deletions(-) create mode 100644 packages/tad-app/csv/smalltxt.csv create mode 100644 packages/tad-app/csv/tabtest.csv diff --git a/packages/tad-app/csv/smalltxt.csv b/packages/tad-app/csv/smalltxt.csv new file mode 100644 index 00000000..ca3eba73 --- /dev/null +++ b/packages/tad-app/csv/smalltxt.csv @@ -0,0 +1,5 @@ +Name,Country +Joe,USA +Bob,USA +Jane,Canada +Mary,UK diff --git a/packages/tad-app/csv/tabtest.csv b/packages/tad-app/csv/tabtest.csv new file mode 100644 index 00000000..1a05a1c7 --- /dev/null +++ b/packages/tad-app/csv/tabtest.csv @@ -0,0 +1,5 @@ +Name,Country,Budget +Joe,USA,1234 +Bob Smith,USA,987654 +Jane Jenkins,Canada,3892 +Mary "Merr E" Jones,UK,9456 diff --git a/packages/tad-app/src/electronRenderMain.tsx b/packages/tad-app/src/electronRenderMain.tsx index 2506003e..8f667636 100644 --- a/packages/tad-app/src/electronRenderMain.tsx +++ b/packages/tad-app/src/electronRenderMain.tsx @@ -100,7 +100,7 @@ async function openFromOpenParams( // TODO: figure out how to initialize based on saved views or different file / table names const init = async () => { const tStart = performance.now(); - log.setLevel(log.levels.DEBUG); + log.setLevel(log.levels.INFO); // debug for more verbosity const appState = new AppState(); const stateRef = mkRef(appState); const [App, listenerId] = refContainer( diff --git a/packages/tadviewer/src/AppState.ts b/packages/tadviewer/src/AppState.ts index a1db7b4f..4e9b8b66 100644 --- a/packages/tadviewer/src/AppState.ts +++ b/packages/tadviewer/src/AppState.ts @@ -28,7 +28,6 @@ export interface AppStateProps { appLoadingTimer: Timer; activity: Activity; showRecordCount: boolean; - showColumnHistograms: boolean; } const defaultAppStateProps: AppStateProps = { @@ -44,7 +43,6 @@ const defaultAppStateProps: AppStateProps = { appLoadingTimer: new Timer(), activity: "None", showRecordCount: true, - showColumnHistograms: true, }; export class AppState extends Immutable.Record(defaultAppStateProps) { @@ -63,5 +61,4 @@ export class AppState extends Immutable.Record(defaultAppStateProps) { public readonly appLoadingTimer!: Timer; public readonly activity!: Activity; public readonly showRecordCount!: boolean; - public readonly showColumnHistograms!: boolean; } diff --git a/packages/tadviewer/src/PivotRequester.ts b/packages/tadviewer/src/PivotRequester.ts index 9ffdc378..2c14c0cc 100644 --- a/packages/tadviewer/src/PivotRequester.ts +++ b/packages/tadviewer/src/PivotRequester.ts @@ -140,7 +140,6 @@ const requestQueryView = async ( baseSchema: reltab.Schema, viewParams: ViewParams, showRecordCount: boolean, - showColumnHistograms: boolean, prevQueryView: QueryView | null | undefined ): Promise => { const schemaCols = baseSchema.columns; @@ -181,7 +180,7 @@ const requestQueryView = async ( let histoMap: ColumnHistogramMap | null = null; if ( - showColumnHistograms && + viewParams.showColumnHistograms && (prevQueryView == null || prevQueryView.baseQuery !== baseQuery || prevQueryView.histoMap == null) @@ -268,7 +267,6 @@ export class PivotRequester { pendingOffset: number; pendingLimit: number; - pendingShowColumnHistograms: boolean; errorCallback?: (e: Error) => void; setLoadingCallback: (loading: boolean) => void; @@ -284,7 +282,6 @@ export class PivotRequester { this.pendingViewParams = null; this.pendingOffset = 0; this.pendingLimit = 0; - this.pendingShowColumnHistograms = appState.showColumnHistograms; this.errorCallback = errorCallback; this.setLoadingCallback = setLoadingCallback || noopSetLoadingCallback; @@ -347,15 +344,14 @@ export class PivotRequester { const { queryView } = viewState; const viewParams = viewState.viewParams; - if ( - viewParams !== this.pendingViewParams || - appState.showColumnHistograms !== this.pendingShowColumnHistograms - ) { + if (viewParams !== this.pendingViewParams) { + /* log.debug( "*** onStateChange: requesting new query: ", viewState.toJS(), this.pendingViewParams?.toJS() ); + */ /* const viewStateJS = viewState.toJS(); const pendingJS = this.pendingViewParams?.toJS(); @@ -373,7 +369,6 @@ export class PivotRequester { // if viewParams are same and only page range differs. const prevViewParams = this.pendingViewParams as ViewParams; this.pendingViewParams = viewParams; - this.pendingShowColumnHistograms = appState.showColumnHistograms; // NOTE! Very important to pass queryView (latest from appState) and not // this.currentQueryView, which may be stale. // TODO: The sequencing and control flow here is really subtle; we should rework this to make @@ -384,7 +379,6 @@ export class PivotRequester { viewState.baseSchema, this.pendingViewParams, appState.showRecordCount, - appState.showColumnHistograms, queryView ); this.pendingQueryRequest = qreq; @@ -409,9 +403,11 @@ export class PivotRequester { .set("queryView", queryView) as ViewState; }) ); + /* log.debug( "*** onStateChange: sending request to satisfy view state update" ); + */ return this.sendDataRequest(stateRef, queryView); }) .catch((err) => { @@ -455,6 +451,7 @@ export class PivotRequester { // No change in view parameters, but check for viewport out of range of // pendingOffset, pendingLimit: const qv: QueryView = this.currentQueryView as any; // Flow misses null check above! + /* log.debug( "*** onStateChange: sending request because paging parameters out of viewport: ", this.pendingOffset, @@ -462,6 +459,7 @@ export class PivotRequester { viewState.viewportTop, viewState.viewportBottom ); + */ this.sendDataRequest(stateRef, qv); } } diff --git a/packages/tadviewer/src/ViewParams.ts b/packages/tadviewer/src/ViewParams.ts index 53b3bf75..809d9201 100644 --- a/packages/tadviewer/src/ViewParams.ts +++ b/packages/tadviewer/src/ViewParams.ts @@ -75,6 +75,7 @@ class FormatDefaults extends Immutable.Record(defaultFormatDefaultProps) { export interface ViewParamsProps { showRoot: boolean; + showColumnHistograms: boolean; displayColumns: Array; // array of column ids to display, in order vpivots: Array; // array of columns to pivot @@ -91,6 +92,7 @@ export interface ViewParamsProps { const defaultViewParamsProps: ViewParamsProps = { showRoot: false, + showColumnHistograms: false, displayColumns: [], vpivots: [], pivotLeafColumn: null, @@ -122,6 +124,7 @@ export class ViewParams implements ViewParamsProps { public readonly showRoot!: boolean; + public readonly showColumnHistograms!: boolean; public readonly displayColumns!: Array; // array of column ids to display, in order public readonly vpivots!: Array; // array of columns to pivot diff --git a/packages/tadviewer/src/actions.ts b/packages/tadviewer/src/actions.ts index 3c5e98ff..650fd35f 100644 --- a/packages/tadviewer/src/actions.ts +++ b/packages/tadviewer/src/actions.ts @@ -74,7 +74,8 @@ export async function stopAppLoadingTimer( export const setQueryView = async ( stateRef: StateRef, dsc: DataSourceConnection, - sqlQuery: string + sqlQuery: string, + showColumnHistograms: boolean ): Promise => { const appState = mutableGet(stateRef); @@ -94,6 +95,7 @@ export const setQueryView = async ( const initialViewParams = new ViewParams({ displayColumns, openPaths, + showColumnHistograms, }); const viewState = new ViewState({ @@ -265,14 +267,32 @@ export const setShowColumnHistograms = ( stateRef: StateRef, showColumnHistograms: boolean ): void => { - update(stateRef, (s) => { - const nextS = s.set( - "showColumnHistograms", - showColumnHistograms - ) as AppState; - return nextS; - }); + update( + stateRef, + vpUpdate( + (viewParams) => + viewParams.set( + "showColumnHistograms", + showColumnHistograms + ) as ViewParams + ) + ); }; +export const toggleShowColumnHistograms = ( + stateRef: StateRef +): void => { + update( + stateRef, + vpUpdate( + (viewParams) => + viewParams.set( + "showColumnHistograms", + !viewParams.showColumnHistograms + ) as ViewParams + ) + ); +}; + export const reorderColumnList = (dstProps: any, srcProps: any) => { console.log("reorderColumnList: ", dstProps, srcProps); diff --git a/packages/tadviewer/src/components/AppPane.tsx b/packages/tadviewer/src/components/AppPane.tsx index 580fcc0f..64a60734 100644 --- a/packages/tadviewer/src/components/AppPane.tsx +++ b/packages/tadviewer/src/components/AppPane.tsx @@ -227,6 +227,7 @@ export const AppPane: React.FunctionComponent = ({ schema={viewState.baseSchema} viewParams={viewState.viewParams} delayedCalcMode={viewState.delayedCalcMode} + embedded={embedded} stateRef={stateRef} /> ); diff --git a/packages/tadviewer/src/components/DataSourceSidebar.tsx b/packages/tadviewer/src/components/DataSourceSidebar.tsx index 85fad703..7129fd30 100644 --- a/packages/tadviewer/src/components/DataSourceSidebar.tsx +++ b/packages/tadviewer/src/components/DataSourceSidebar.tsx @@ -114,12 +114,14 @@ export const DataSourceSidebar: React.FC = ({ sourceId, path: ["."], }; + /* log.debug( "DataSourceSidebar: creating root node for", sourceIdStr, rootPath, rootNode ); + */ rootTreeNode = dsNodeTreeNode(dsc, rootPath, rootNode); nextNodeMap[sourceIdStr] = rootTreeNode; if (rootNode.isContainer) { diff --git a/packages/tadviewer/src/components/GridPane.tsx b/packages/tadviewer/src/components/GridPane.tsx index 30d2ef41..f63cc976 100644 --- a/packages/tadviewer/src/components/GridPane.tsx +++ b/packages/tadviewer/src/components/GridPane.tsx @@ -325,16 +325,14 @@ export interface GridPaneProps { embedded: boolean; } -const getGridOptions = ( - showColumnHistograms: boolean, - viewState: ViewState -) => { +const getGridOptions = (viewState: ViewState) => { const { queryView } = viewState; const histoCount = queryView?.histoMap ? Object.keys(queryView.histoMap).length : 0; - const showHeaderRow = showColumnHistograms && histoCount > 0; + const showHeaderRow = + viewState.viewParams.showColumnHistograms && histoCount > 0; const gridOptions = { ...baseGridOptions, showHeaderRow, @@ -345,7 +343,7 @@ const getGridOptions = ( const getGridOptionsFromStateRef = (stateRef: StateRef) => { const appState = mutableGet(stateRef); - return getGridOptions(appState.showColumnHistograms, appState.viewState); + return getGridOptions(appState.viewState); }; // escape tabs by placing string in quotes @@ -429,6 +427,7 @@ const createGrid = ( const appState = mutableGet(stateRef); const viewState = appState.viewState; const { queryView } = viewState; + console.log("onHeaderRowCellRendered: ", column); if (queryView && queryView.histoMap && queryView.histoMap[column.id]) { const histo = queryView.histoMap[column.id]; const colType = viewState.baseSchema.columnType(column.id); @@ -441,6 +440,8 @@ const createGrid = ( /> ); node.classList.add("slick-editable"); + } else { + console.log("*** no histo for column: ", column.id); } }); @@ -556,11 +557,7 @@ const getGridCols = (gs: GridState, viewState: ViewState) => { /* * update grid from dataView */ -const updateGrid = ( - gs: GridState, - viewState: ViewState, - showColumnHistograms: boolean -) => { +const updateGrid = (gs: GridState, viewState: ViewState) => { const { viewParams, dataView } = viewState; if (dataView == null) return; @@ -569,7 +566,8 @@ const updateGrid = ( const grid = gs.grid; - const gridOptions = getGridOptions(showColumnHistograms, viewState); + const gridOptions = getGridOptions(viewState); + // console.log("updateGrid: gridOptions: ", gridOptions); grid.setOptions(gridOptions); grid.setHeaderRowVisibility(gridOptions.showHeaderRow); @@ -588,6 +586,7 @@ const updateGrid = ( grid.invalidateAllRows(); grid.updateRowCount(); grid.render(); + grid.resizeCanvas(); }; const createGridState = ( @@ -634,13 +633,15 @@ const RawGridPane: React.FunctionComponent = ({ const [gridState, setGridState] = useState(null); const viewStateRef = useRef(viewState); - const prevShowColumnHistograms = useRef(appState.showColumnHistograms); + const prevShowColumnHistograms = useRef( + viewState.viewParams.showColumnHistograms + ); viewStateRef.current = viewState; const dataView = viewState.dataView; - const { showColumnHistograms } = appState; + const { showColumnHistograms } = viewState.viewParams; // log.debug("RawGridPane: ", appState.toJS(), viewState.toJS()); @@ -649,10 +650,12 @@ const RawGridPane: React.FunctionComponent = ({ // The extra check here for prevShowColumnHistograms is a workaround // for an apparent bug in SlickGrid where it doesn't seem to re-render // correctly when we dynamically change the showHeaderRow option on the grid. + const histoMap = viewState.queryView?.histoMap; if ( gs === null || - prevShowColumnHistograms.current !== showColumnHistograms + (prevShowColumnHistograms.current !== showColumnHistograms && histoMap) ) { + // log.debug("RawGridPane: creating grid state"); gs = createGridState( stateRef, viewStateRef, @@ -666,15 +669,15 @@ const RawGridPane: React.FunctionComponent = ({ } gs.grid.resizeCanvas(); setGridState(gs); + // log.debug("RawGridPane: done creating grid state"); + prevShowColumnHistograms.current = showColumnHistograms; } - // log.debug("GridPane effect: ", prevDataView, dataView); if (dataView != null) { // log.debug("RawGridPane: updating grid"); - updateGrid(gs, viewStateRef.current, showColumnHistograms); + updateGrid(gs, viewStateRef.current); } else { // log.debug("RawGridPane: no view change, skipping grid update"); } - prevShowColumnHistograms.current = showColumnHistograms; }, [dataView, gridState, showColumnHistograms]); const handleGridResize = (entries: ResizeEntry[]) => { diff --git a/packages/tadviewer/src/components/PivotSidebar.tsx b/packages/tadviewer/src/components/PivotSidebar.tsx index 0786b8e1..8fc7960c 100644 --- a/packages/tadviewer/src/components/PivotSidebar.tsx +++ b/packages/tadviewer/src/components/PivotSidebar.tsx @@ -21,6 +21,7 @@ export interface PivotSidebarProps { viewParams: ViewParams; delayedCalcMode: boolean; onColumnClick?: (cid: string) => void; + embedded: boolean; stateRef: StateRef; } @@ -30,6 +31,7 @@ export const PivotSidebar: React.FC = ({ viewParams, delayedCalcMode, onColumnClick, + embedded, stateRef, }) => { const onLeafColumnSelect = (event: React.ChangeEvent) => { @@ -75,11 +77,21 @@ export const PivotSidebar: React.FC = ({ ); + const columnHistoCheckElem = ( + actions.toggleShowColumnHistograms(stateRef)} + label="Show Numeric Column Histograms" + /> + ); + return (
General
+ {columnHistoCheckElem} { if (appStateRef != null && pivotRequester != null) { - actions.setQueryView(appStateRef, dsConn, baseSqlQuery); + actions.setQueryView( + appStateRef, + dsConn, + baseSqlQuery, + showColumnHistograms + ); log.debug("**** set app view to base query"); } }, [baseSqlQuery, pivotRequester, appStateRef]);