michael-s-molina commented on code in PR #42088:
URL: https://github.com/apache/superset/pull/42088#discussion_r3646986962
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -368,75 +395,202 @@ export default function TableChart<D extends DataRecord
= DataRecord>(
[emitCrossFilters, setDataMask, timeGrain, timestampFormatter],
);
+ const drillColumns = isUsingTimeComparison
+ ? (filteredColumns as InputColumn[])
+ : (columns as InputColumn[]);
+
+ const handleContextMenu = useCallback(
+ (event: CellContextMenuEvent) => {
+ if (!onContextMenu || isRawRecords || !event.column || !event.data) {
+ return;
+ }
+ const nativeEvent = event.event as MouseEvent | null | undefined;
+ if (!nativeEvent) return;
+ nativeEvent.preventDefault();
+ nativeEvent.stopPropagation();
+
+ const rowData = event.data as Record<string, DataRecordValue>;
+ const key = event.column.getColId();
+ const cellValue = event.value as DataRecordValue;
+ const colDef = event.column.getColDef();
+ const isMetric = Boolean(
+ colDef.context?.isMetric || colDef.context?.isPercentMetric,
+ );
+
+ const drillToDetailFilters: BinaryQueryObjectFilterClause[] = [];
+ drillColumns.forEach(col => {
+ if (col.isMetric || col.isPercentMetric) return;
+ const dataRecordValue = rowData[col.key];
+
+ if (
+ dataRecordValue == null ||
+ (dataRecordValue instanceof DateWithFormatter &&
+ dataRecordValue.input == null)
+ ) {
+ drillToDetailFilters.push({
+ col: col.key,
+ op: 'IS NULL' as any,
+ val: null,
+ });
+ } else if (col.dataType === GenericDataType.Temporal && timeGrain) {
+ const startTime =
+ dataRecordValue instanceof Date
+ ? dataRecordValue
+ : new Date(dataRecordValue as string | number);
+
+ if (Number.isNaN(startTime.getTime())) {
+ // Malformed temporal value: fall back to an equality filter
+ // instead of building a TEMPORAL_RANGE, since toISOString()
+ // throws on an Invalid Date and would crash the context menu.
+ const sanitizedValue = extractTextFromHTML(dataRecordValue);
+ drillToDetailFilters.push({
+ col: col.key,
+ op: '==',
+ val: sanitizedValue as string | number | boolean,
+ formattedVal: formatColumnValue(col, sanitizedValue)[1],
+ });
+ } else {
+ const [rangeStartTime, rangeEndTime] = getTimeRangeFromGranularity(
+ startTime,
+ timeGrain,
+ );
+ const timeRangeValue = `${rangeStartTime.toISOString()} :
${rangeEndTime.toISOString()}`;
+
+ drillToDetailFilters.push({
+ col: col.key,
+ op: 'TEMPORAL_RANGE',
+ val: timeRangeValue,
+ grain: timeGrain,
+ formattedVal: formatColumnValue(col, dataRecordValue)[1],
+ });
+ }
+ } else {
+ const sanitizedValue = extractTextFromHTML(dataRecordValue);
+ drillToDetailFilters.push({
+ col: col.key,
+ op: '==',
+ val: sanitizedValue as string | number | boolean,
+ formattedVal: formatColumnValue(col, sanitizedValue)[1],
+ });
+ }
+ });
+
+ const isCellValueNull =
+ cellValue == null ||
+ (cellValue instanceof DateWithFormatter && cellValue.input == null);
+
+ onContextMenu(nativeEvent.clientX, nativeEvent.clientY, {
+ drillToDetail: drillToDetailFilters,
+ crossFilter: isMetric
+ ? undefined
+ : getCrossFilterDataMask({
+ key,
+ value: cellValue,
+ filters,
+ timeGrain,
+ isActiveFilterValue,
+ timestampFormatter,
+ }),
+ drillBy: isMetric
+ ? undefined
+ : {
+ filters: [
+ isCellValueNull
+ ? { col: key, op: 'IS NULL' as any, val: null }
+ : {
+ col: key,
+ op: '==' as any,
+ val: extractTextFromHTML(cellValue),
+ },
+ ],
+ groupbyFieldName: 'groupby',
+ },
+ });
+ },
+ [
+ onContextMenu,
+ isRawRecords,
+ drillColumns,
+ timeGrain,
+ filters,
+ isActiveFilterValue,
+ timestampFormatter,
+ ],
+ );
+
const handleServerPaginationChange = useCallback(
(pageNumber: number, pageSize: number) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
currentPage: pageNumber,
pageSize,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask],
+ [writeOwnState],
);
const handlePageSizeChange = useCallback(
(pageSize: number) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
currentPage: 0,
pageSize,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask],
+ [writeOwnState],
);
const handleChangeSearchCol = (searchCol: string) => {
- if (!isEqual(searchCol, serverPaginationData?.searchColumn)) {
- const modifiedOwnState = {
- ...serverPaginationData,
+ if (!isEqual(searchCol, ownStateRef.current?.searchColumn)) {
+ writeOwnState({
searchColumn: searchCol,
searchText: '',
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
}
};
const handleSearch = useCallback(
(searchText: string) => {
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
searchColumn:
- serverPaginationData?.searchColumn || searchOptions[0]?.value,
+ (ownStateRef.current?.searchColumn as string | undefined) ||
+ searchOptions[0]?.value,
searchText,
currentPage: 0, // Reset to first page when searching
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
},
- [setDataMask, searchOptions],
+ [writeOwnState, searchOptions],
);
const handleSortByChange = useCallback(
(sortBy: SortByItem[]) => {
if (!serverPagination) return;
- const modifiedOwnState = {
- ...serverPaginationData,
+ writeOwnState({
sortBy,
lastFilteredColumn: undefined,
lastFilteredInputPosition: undefined,
- };
- updateTableOwnState(setDataMask, modifiedOwnState);
+ });
+ },
+ [writeOwnState, serverPagination],
+ );
+
+ // Feeds the "Export Current View" menu item (EXPORT_CURRENT_VIEW behavior),
+ // mirroring Table V1's clientView snapshot on ownState. Written through
+ // writeOwnState (rather than spreading serverPaginationData directly)
+ // because onModelUpdated can fire with a stale closure relative to other
+ // ownState writers (e.g. a just-applied filter), and updateTableOwnState
+ // replaces ownState wholesale.
+ const handleClientViewChange = useCallback(
+ (clientView: ClientViewSnapshot) => {
+ writeOwnState({ clientView });
Review Comment:
Confirmed and fixed in 387f844bd8: `convertAgGridStateToOwnState` now
returns `{}` immediately when `serverPagination` is false, so client-mode
filter/sort/column changes no longer fold into ownState and trigger the
requery/remount that was stripping the just-applied filter. Kept the gate
inside the plugin's converter (with a new optional `serverPagination` flag on
`AgGridChartState`) rather than in the shared Explore/Dashboard fold logic, so
it stays self-contained to ag-grid-table.
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTableChart.tsx:
##########
@@ -493,6 +662,7 @@ export default function TableChart<D extends DataRecord =
DataRecord>(
width={width}
onColumnStateChange={handleColumnStateChange}
Review Comment:
Confirmed and fixed in 387f844bd8, same root cause as the sibling comment on
this review: gated `convertAgGridStateToOwnState` on `serverPagination` so the
chartState->ownState fold (via `createOwnStateWithChartState`) is a no-op in
client mode, since none of sortBy/columnOrder/sqlClauses/pageSize/currentPage
are needed by the backend query when AG Grid handles everything locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
