Skip to content

Commit

Permalink
move showColumnHistograms into viewState where it belongs.
Browse files Browse the repository at this point in the history
  • Loading branch information
antonycourtney committed Oct 17, 2023
1 parent e26f303 commit 6e85c23
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 42 deletions.
5 changes: 5 additions & 0 deletions packages/tad-app/csv/smalltxt.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Name,Country
Joe,USA
Bob,USA
Jane,Canada
Mary,UK
5 changes: 5 additions & 0 deletions packages/tad-app/csv/tabtest.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Name,Country,Budget
Joe,USA,1234
Bob Smith,USA,987654
Jane Jenkins,Canada,3892
Mary "Merr E" Jones,UK,9456
2 changes: 1 addition & 1 deletion packages/tad-app/src/electronRenderMain.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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<AppState, AppPaneBaseProps>(
Expand Down
3 changes: 0 additions & 3 deletions packages/tadviewer/src/AppState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ export interface AppStateProps {
appLoadingTimer: Timer;
activity: Activity;
showRecordCount: boolean;
showColumnHistograms: boolean;
}

const defaultAppStateProps: AppStateProps = {
Expand All @@ -44,7 +43,6 @@ const defaultAppStateProps: AppStateProps = {
appLoadingTimer: new Timer(),
activity: "None",
showRecordCount: true,
showColumnHistograms: true,
};

export class AppState extends Immutable.Record(defaultAppStateProps) {
Expand All @@ -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;
}
18 changes: 8 additions & 10 deletions packages/tadviewer/src/PivotRequester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,6 @@ const requestQueryView = async (
baseSchema: reltab.Schema,
viewParams: ViewParams,
showRecordCount: boolean,
showColumnHistograms: boolean,
prevQueryView: QueryView | null | undefined
): Promise<QueryView> => {
const schemaCols = baseSchema.columns;
Expand Down Expand Up @@ -181,7 +180,7 @@ const requestQueryView = async (

let histoMap: ColumnHistogramMap | null = null;
if (
showColumnHistograms &&
viewParams.showColumnHistograms &&
(prevQueryView == null ||
prevQueryView.baseQuery !== baseQuery ||
prevQueryView.histoMap == null)
Expand Down Expand Up @@ -268,7 +267,6 @@ export class PivotRequester {

pendingOffset: number;
pendingLimit: number;
pendingShowColumnHistograms: boolean;
errorCallback?: (e: Error) => void;
setLoadingCallback: (loading: boolean) => void;

Expand All @@ -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;

Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand All @@ -384,7 +379,6 @@ export class PivotRequester {
viewState.baseSchema,
this.pendingViewParams,
appState.showRecordCount,
appState.showColumnHistograms,
queryView
);
this.pendingQueryRequest = qreq;
Expand All @@ -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) => {
Expand Down Expand Up @@ -455,13 +451,15 @@ 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,
this.pendingLimit,
viewState.viewportTop,
viewState.viewportBottom
);
*/
this.sendDataRequest(stateRef, qv);
}
}
Expand Down
3 changes: 3 additions & 0 deletions packages/tadviewer/src/ViewParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class FormatDefaults extends Immutable.Record(defaultFormatDefaultProps) {

export interface ViewParamsProps {
showRoot: boolean;
showColumnHistograms: boolean;
displayColumns: Array<string>; // array of column ids to display, in order

vpivots: Array<string>; // array of columns to pivot
Expand All @@ -91,6 +92,7 @@ export interface ViewParamsProps {

const defaultViewParamsProps: ViewParamsProps = {
showRoot: false,
showColumnHistograms: false,
displayColumns: [],
vpivots: [],
pivotLeafColumn: null,
Expand Down Expand Up @@ -122,6 +124,7 @@ export class ViewParams
implements ViewParamsProps
{
public readonly showRoot!: boolean;
public readonly showColumnHistograms!: boolean;
public readonly displayColumns!: Array<string>; // array of column ids to display, in order

public readonly vpivots!: Array<string>; // array of columns to pivot
Expand Down
36 changes: 28 additions & 8 deletions packages/tadviewer/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ export async function stopAppLoadingTimer(
export const setQueryView = async (
stateRef: StateRef<AppState>,
dsc: DataSourceConnection,
sqlQuery: string
sqlQuery: string,
showColumnHistograms: boolean
): Promise<void> => {
const appState = mutableGet(stateRef);

Expand All @@ -94,6 +95,7 @@ export const setQueryView = async (
const initialViewParams = new ViewParams({
displayColumns,
openPaths,
showColumnHistograms,
});

const viewState = new ViewState({
Expand Down Expand Up @@ -265,14 +267,32 @@ export const setShowColumnHistograms = (
stateRef: StateRef<AppState>,
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<AppState>
): void => {
update(
stateRef,
vpUpdate(
(viewParams) =>
viewParams.set(
"showColumnHistograms",
!viewParams.showColumnHistograms
) as ViewParams
)
);
};

export const reorderColumnList = (dstProps: any, srcProps: any) => {
console.log("reorderColumnList: ", dstProps, srcProps);

Expand Down
1 change: 1 addition & 0 deletions packages/tadviewer/src/components/AppPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ export const AppPane: React.FunctionComponent<AppPaneProps> = ({
schema={viewState.baseSchema}
viewParams={viewState.viewParams}
delayedCalcMode={viewState.delayedCalcMode}
embedded={embedded}
stateRef={stateRef}
/>
);
Expand Down
2 changes: 2 additions & 0 deletions packages/tadviewer/src/components/DataSourceSidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,14 @@ export const DataSourceSidebar: React.FC<DataSourceSidebarProps> = ({
sourceId,
path: ["."],
};
/*
log.debug(
"DataSourceSidebar: creating root node for",
sourceIdStr,
rootPath,
rootNode
);
*/
rootTreeNode = dsNodeTreeNode(dsc, rootPath, rootNode);
nextNodeMap[sourceIdStr] = rootTreeNode;
if (rootNode.isContainer) {
Expand Down
39 changes: 21 additions & 18 deletions packages/tadviewer/src/components/GridPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -345,7 +343,7 @@ const getGridOptions = (
const getGridOptionsFromStateRef = (stateRef: StateRef<AppState>) => {
const appState = mutableGet(stateRef);

return getGridOptions(appState.showColumnHistograms, appState.viewState);
return getGridOptions(appState.viewState);
};

// escape tabs by placing string in quotes
Expand Down Expand Up @@ -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);
Expand All @@ -441,6 +440,8 @@ const createGrid = (
/>
);
node.classList.add("slick-editable");
} else {
console.log("*** no histo for column: ", column.id);
}
});

Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand All @@ -588,6 +586,7 @@ const updateGrid = (
grid.invalidateAllRows();
grid.updateRowCount();
grid.render();
grid.resizeCanvas();
};

const createGridState = (
Expand Down Expand Up @@ -634,13 +633,15 @@ const RawGridPane: React.FunctionComponent<GridPaneProps> = ({
const [gridState, setGridState] = useState<GridState | null>(null);
const viewStateRef = useRef<ViewState>(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());

Expand All @@ -649,10 +650,12 @@ const RawGridPane: React.FunctionComponent<GridPaneProps> = ({
// 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,
Expand All @@ -666,15 +669,15 @@ const RawGridPane: React.FunctionComponent<GridPaneProps> = ({
}
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[]) => {
Expand Down
Loading

0 comments on commit 6e85c23

Please sign in to comment.