amaannawab923 commented on code in PR #42088:
URL: https://github.com/apache/superset/pull/42088#discussion_r3659455174


##########
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:
   `clientView` is written into ownState via `setDataMask` on every 
filtered-row change. in client mode on a dashboard, that makes the chart 
re-query on each filter/sort (a `/chart/data` per interaction) and the refetch 
remounts the grid, dropping the live filter. since it's only for Export Current 
View (which already falls back to a backend export when `ownState.clientView` 
is absent), it probably shouldn't go through the query-triggering ownState.



-- 
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]

Reply via email to