codeant-ai-for-open-source[bot] commented on code in PR #37784: URL: https://github.com/apache/superset/pull/37784#discussion_r2778111569
########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/hooks/useSticky.tsx: ########## @@ -0,0 +1,451 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + Children, + cloneElement, + useRef, + useMemo, + useLayoutEffect, + useCallback, + ReactNode, + ReactElement, + ComponentPropsWithRef, + CSSProperties, + UIEventHandler, +} from 'react'; +import { TableInstance, Hooks } from 'react-table'; +import getScrollBarSize from '../utils/getScrollBarSize'; +import needScrollBar from '../utils/needScrollBar'; +import useMountedMemo from '../utils/useMountedMemo'; + +type ReactElementWithChildren< + T extends keyof JSX.IntrinsicElements, + C extends ReactNode = ReactNode, +> = ReactElement<ComponentPropsWithRef<T> & { children: C }, T>; + +type Th = ReactElementWithChildren<'th'>; +type Td = ReactElementWithChildren<'td'>; +type TrWithTh = ReactElementWithChildren<'tr', Th[]>; +type TrWithTd = ReactElementWithChildren<'tr', Td[]>; +type Thead = ReactElementWithChildren<'thead', TrWithTh>; +type Tbody = ReactElementWithChildren<'tbody', TrWithTd>; +type Tfoot = ReactElementWithChildren<'tfoot', TrWithTd>; +type Col = ReactElementWithChildren<'col', null>; +type ColGroup = ReactElementWithChildren<'colgroup', Col>; + +export type Table = ReactElementWithChildren< + 'table', + (Thead | Tbody | Tfoot | ColGroup)[] +>; +export type TableRenderer = () => Table; +export type GetTableSize = () => Partial<StickyState> | undefined; +export type SetStickyState = (size?: Partial<StickyState>) => void; + +export enum ReducerActions { + Init = 'init', // this is from global reducer + SetStickyState = 'setStickyState', +} + +export type ReducerAction< + T extends string, + P extends Record<string, unknown>, +> = P & { type: T }; + +export type ColumnWidths = number[]; + +export interface StickyState { + width?: number; // maximum full table width + height?: number; // maximum full table height + realHeight?: number; // actual table viewport height (header + scrollable area) + bodyHeight?: number; // scrollable area height + tableHeight?: number; // the full table height + columnWidths?: ColumnWidths; + hasHorizontalScroll?: boolean; + hasVerticalScroll?: boolean; + rendering?: boolean; + setStickyState?: SetStickyState; +} + +export interface UseStickyTableOptions { + getTableSize?: GetTableSize; +} + +export interface UseStickyInstanceProps { + // manipulate DOMs in <table> to make the header sticky + wrapStickyTable: (renderer: TableRenderer) => ReactNode; + // update or recompute the sticky table size + setStickyState: SetStickyState; +} + +export type UseStickyState = { + sticky: StickyState; +}; + +const sum = (a: number, b: number) => a + b; +const mergeStyleProp = ( + node: ReactElement<{ style?: CSSProperties }>, + style: CSSProperties, +) => ({ + style: { + ...node.props.style, + ...style, + }, +}); +const fixedTableLayout: CSSProperties = { tableLayout: 'fixed' }; + +/** + * An HOC for generating sticky header and fixed-height scrollable area + */ +function StickyWrap({ + sticky = {}, + width: maxWidth, + height: maxHeight, + children: table, + setStickyState, +}: { + width: number; + height: number; + setStickyState: SetStickyState; + children: Table; + sticky?: StickyState; // current sticky element sizes +}) { + if (!table || table.type !== 'table') { + throw new Error('<StickyWrap> must have only one <table> element as child'); + } + let thead: Thead | undefined; + let tbody: Tbody | undefined; + let tfoot: Tfoot | undefined; + + Children.forEach(table.props.children, node => { + if (!node) { + return; + } + if (node.type === 'thead') { + thead = node; + } else if (node.type === 'tbody') { + tbody = node; + } else if (node.type === 'tfoot') { + tfoot = node; + } + }); + if (!thead || !tbody) { + throw new Error( + '<table> in <StickyWrap> must contain both thead and tbody.', + ); + } + const columnCount = useMemo(() => { + const headerRows = Children.toArray( + thead?.props.children, + ).pop() as TrWithTh; + return headerRows.props.children.length; Review Comment: **Suggestion:** If the table header has no rows, `Children.toArray(...).pop()` returns `undefined` and accessing `.props.children.length` will throw, so `columnCount` computation needs to handle an empty header safely. [possible bug] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Sticky header crashes when `<thead>` has zero rows. - ⚠️ Remita table chart unusable for misconfigured empty headers. ``` </details> ```suggestion ).pop() as TrWithTh | undefined; return headerRows?.props.children.length ?? 0; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. A React table instance uses `useSticky` (exported at `useSticky.tsx:419-450`), which registers `useInstance` with react-table hooks so that the component tree eventually calls `wrapStickyTable` and renders `<StickyWrap>` with the provided `table` element. 2. The `table` element passed into `<StickyWrap>` (`useSticky.tsx:120-127`) has a `<thead>` child (`thead` is set in the `Children.forEach` loop at `useSticky.tsx:135-145`), but that `<thead>` renders with no header rows (no `<tr>` children) in `thead.props.children`. 3. During render of `<StickyWrap>`, the memoized `columnCount` computation at `useSticky.tsx:152-157` executes: `Children.toArray(thead?.props.children)` returns an empty array, and `.pop()` at line 154 produces `undefined` for `headerRows`. 4. The next line (`headerRows.props.children.length`) attempts to read `props` from `undefined`, throwing a runtime error (`TypeError: Cannot read properties of undefined (reading 'props')`) which prevents the sticky header from rendering. In the current DataTable/react-table configuration, header rows are always produced from column definitions, so having a `<thead>` with no rows is an invalid/misconfigured state, making this crash unlikely in normal Superset dashboards. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/hooks/useSticky.tsx **Line:** 155:156 **Comment:** *Possible Bug: If the table header has no rows, `Children.toArray(...).pop()` returns `undefined` and accessing `.props.children.length` will throw, so `columnCount` computation needs to handle an empty header safely. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=f4fc3948f9cdecf6b07390e913240204431738221f15855c6e900b09c6a4e731&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=f4fc3948f9cdecf6b07390e913240204431738221f15855c6e900b09c6a4e731&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/BulkActions.tsx: ########## @@ -0,0 +1,338 @@ +import React, { memo } from 'react'; +import { Dropdown } from '@superset-ui/chart-controls'; +import { Button, Space, Tag } from '@superset-ui/core/components'; +import { useTheme } from '@apache-superset/core/ui'; +import { DownOutlined } from '@ant-design/icons'; +import { + PlusOutlined, + EditOutlined, + DeleteOutlined, + EyeOutlined, + LinkOutlined, + CheckOutlined, + KeyOutlined, + TagOutlined, + MoreOutlined, +} from '@ant-design/icons'; +import { BulkAction } from '.'; + +// Helper: read RLS extra rules primarily from window; fallback to decoding a stored JWT +const getRlsExtraRules = (): Record<string, any> => { + try { + const fromWindow = (window as any).rls_extra_rules; + if (fromWindow && typeof fromWindow === 'object') return fromWindow; + + const storedToken = + (typeof localStorage !== 'undefined' && localStorage.getItem('access_token')) || + (typeof sessionStorage !== 'undefined' && sessionStorage.getItem('access_token')) || + ''; + if (storedToken && storedToken.includes('.')) { + try { + const parts = storedToken.split('.'); + let payload = parts[1] || ''; + // Handle base64url -> base64 conversion + payload = payload.replace(/-/g, '+').replace(/_/g, '/'); + while (payload.length % 4) payload += '='; + const decoded = JSON.parse(atob(payload)); + if (decoded && typeof decoded === 'object' && decoded.rls_extra_rules) { + return decoded.rls_extra_rules as Record<string, any>; + } + } catch { + // ignore token decode errors + } + } + } catch { + // no-op + } + return {}; +}; + +// Evaluate RLS visibility conditions (ALL must pass - AND logic) +const evaluateRlsVisibilityConditions = ( + conditions: Array<{ rlsKey: string; operator: string; value: any }> | undefined, +): boolean => { + if (!conditions || conditions.length === 0) { + return true; // No conditions means visible + } + + const rlsRules = getRlsExtraRules(); + + for (const condition of conditions) { + const { rlsKey, operator, value: condValue } = condition; + const rlsValue = rlsRules[rlsKey]; + + let passes = false; + switch (operator) { + case "==": + passes = rlsValue == condValue; + break; + case "!=": + passes = rlsValue != condValue; + break; + case "IN": { + const list = String(condValue).split(",").map((s) => s.trim()); + passes = list.includes(String(rlsValue)); + break; + } + case "NOT IN": { + const list = String(condValue).split(",").map((s) => s.trim()); + passes = !list.includes(String(rlsValue)); + break; + } + default: + passes = true; + } + + // If any condition fails, return false (AND logic) + if (!passes) { + return false; + } + } + + return true; // All conditions passed +}; + +const styles = ` + .bulk-actions-container { + display: flex; + width: fit-content; + align-items: center; + padding: 8px; + justify-content: flex-end; + } + + .btn-group { display: flex; gap: 8px; align-items: center; } + .selection-badge { + display: inline-flex; + align-items: center; + padding: 4px 8px; + margin-right: 2px; + border-radius: 8px; + font-size: 12px; + background-color: #eee; + color: #666666; + font-weight: 500; + cursor: pointer; + transition: background-color 0.3s ease, color 0.3s ease, transform 0.2s ease; + } + + .selection-badge:hover { + background-color: #ddd; + color: #333; + transform: scale(1.05); + } + + .selection-badge:active { + transform: scale(0.95); + background-color: #ccc; + } + + .btn { margin-left: 4px; } +`; + +export interface BulkActionProps { + initialSelectedRows: Map<string, any>; + bulkActionLabel?: string; + actions: { + split: Set<BulkAction>; + nonSplit: Set<BulkAction>; + }; + onActionClick: (actionKey: string) => void; + onClearSelection?: () => void; + showSplitInSliceHeader: boolean; + value?: string; + rowId?: string | number; + sliceId?: string | number; + onInvertSelection?: () => void; +} + +export const BulkActions: React.FC<BulkActionProps> = memo(({ + initialSelectedRows, + bulkActionLabel, + actions, + onActionClick, + onClearSelection, + showSplitInSliceHeader, + sliceId, + onInvertSelection, + }) => { + // ✅ FIX: Use props directly instead of copying to local state + // This eliminates race conditions between parent and child state updates + const selectedRows = initialSelectedRows; + const hasSelection = selectedRows?.size > 0; + + // Convert Sets to Arrays for filtering. + const splitActions = actions?.split ? Array.from(actions.split) : []; + const nonSplitActions = actions?.nonSplit ? Array.from(actions.nonSplit) : []; + + const theme = useTheme(); + + // Filter actions that should be visible + // Split actions: show in table only if NOT globally moved to slice header + // Non-split actions: show in table only if individually NOT marked for slice header + const visibleActions = { + // Split actions: controlled by global showSplitInSliceHeader flag + dropdown: !showSplitInSliceHeader + ? splitActions.filter(action => { + // Check selection-based visibility + const selectionVisible = action.visibilityCondition === 'all' || + action.visibilityCondition === 'selected' || + (action.visibilityCondition === 'unselected' && !hasSelection) || + !action.visibilityCondition; + + // Check RLS visibility conditions (ALL must pass) + const rlsVisible = evaluateRlsVisibilityConditions(action.rlsVisibilityConditions); + + return selectionVisible && rlsVisible; + }) + : [], + + // Non-split actions: controlled by individual showInSliceHeader property + buttons: nonSplitActions.filter(action => { + // Check if should show in slice header + if (action.showInSliceHeader) return false; + + // Check selection-based visibility + const selectionVisible = action.visibilityCondition === 'all' || + action.visibilityCondition === 'selected' || + (action.visibilityCondition === 'unselected' && !hasSelection) || + !action.visibilityCondition; Review Comment: **Suggestion:** The selection-based visibility logic treats the "selected" visibility condition the same as "all", so actions configured to show only when rows are selected will appear even when there is no selection, which can lead to actions being available in invalid contexts. Adjust the visibility check so that "selected" actions are only visible when there is at least one selected row, while keeping "all" and "unselected" behavior unchanged. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Selected-only bulk actions visible with no selected rows. - ⚠️ Users can trigger actions expecting a non-empty selection. ``` </details> ```suggestion const isSelectionVisible = (visibilityCondition?: string) => { if (!visibilityCondition || visibilityCondition === 'all') { return true; } if (visibilityCondition === 'selected') { return hasSelection; } if (visibilityCondition === 'unselected') { return !hasSelection; } return true; }; const visibleActions = { // Split actions: controlled by global showSplitInSliceHeader flag dropdown: !showSplitInSliceHeader ? splitActions.filter(action => { // Check selection-based visibility const selectionVisible = isSelectionVisible(action.visibilityCondition); // Check RLS visibility conditions (ALL must pass) const rlsVisible = evaluateRlsVisibilityConditions(action.rlsVisibilityConditions); return selectionVisible && rlsVisible; }) : [], // Non-split actions: controlled by individual showInSliceHeader property buttons: nonSplitActions.filter(action => { // Check if should show in slice header if (action.showInSliceHeader) return false; // Check selection-based visibility const selectionVisible = isSelectionVisible(action.visibilityCondition); ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Render the `BulkActions` component from `superset-frontend/plugins/plugin-chart-remita-table/src/BulkActions.tsx:149` with `initialSelectedRows` as an empty `Map()` (no rows selected) and a `BulkAction` in `actions.nonSplit` or `actions.split` whose `visibilityCondition` is set to `'selected'`. 2. During render, `BulkActions` computes `hasSelection` at `BulkActions.tsx:161` as `selectedRows?.size > 0`, which evaluates to `false` for an empty `Map()`. 3. The visibility filtering logic at `BulkActions.tsx:170-205` sets `selectionVisible` to `true` for this action because the condition `action.visibilityCondition === 'selected'` is used without checking `hasSelection`, so the `'selected'` branch ignores the actual selection state. 4. As a result, the action configured to show only when rows are selected is still included in `visibleActions.dropdown`/`visibleActions.buttons` and rendered as a clickable button in the header even when no rows are selected, exposing an action in an invalid context. This behavior follows directly from the code and does not depend on any external callers. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/BulkActions.tsx **Line:** 173:199 **Comment:** *Logic Error: The selection-based visibility logic treats the "selected" visibility condition the same as "all", so actions configured to show only when rows are selected will appear even when there is no selection, which can lead to actions being available in invalid contexts. Adjust the visibility check so that "selected" actions are only visible when there is at least one selected row, while keeping "all" and "unselected" behavior unchanged. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=f48099cf22caf01a3b878c4932b3fce7cc3ff51be8bb110d9915c36093025a01&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=f48099cf22caf01a3b878c4932b3fce7cc3ff51be8bb110d9915c36093025a01&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts: ########## @@ -0,0 +1,37 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { Row } from 'react-table'; + +export const sortAlphanumericCaseInsensitive = <D extends {}>( + rowA: Row<D>, + rowB: Row<D>, + columnId: string, +) => { + const valueA = rowA.values[columnId]; + const valueB = rowB.values[columnId]; + + if (!valueA || typeof valueA !== 'string') { + return -1; + } + if (!valueB || typeof valueB !== 'string') { Review Comment: **Suggestion:** The use of a generic falsy check (`!valueA` / `!valueB`) treats empty strings and zero as "missing" values and always sorts them to the beginning or end, which is inconsistent with how valid values should be ordered and can surprise callers relying on proper string sorting. Replace the falsy checks with explicit null/undefined checks so that valid but falsy values like empty strings are still compared normally. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Remita table columns mis-handle empty-string ordering when sorted. - ⚠️ Inconsistent behavior between null and empty string cell values. ``` </details> ```suggestion if (valueA == null || typeof valueA !== 'string') { return -1; } if (valueB == null || typeof valueB !== 'string') { ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Import the comparator from `superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts`. 2. Construct two `Row`-like objects where `rowA.values['col'] === ''` (empty string) and `rowB.values['col'] === 'A'`, matching the `Row<D>` shape expected at lines 22–28. 3. Call `sortAlphanumericCaseInsensitive(rowA, rowB, 'col')` from this file (lines 22–36); in the body, at line 30, `!valueA` evaluates to `true` for the empty string, so the function returns `-1` without reaching `localeCompare`. 4. Observe that an empty string, which is a valid string value, is always treated as "missing" and forced to the beginning of the sort order instead of being compared lexicographically, which can be seen directly in this function without needing other callers; non-string falsy values (like `0`) are already filtered by the `typeof !== 'string'` check so the main real-world effect is on empty strings. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/sortAlphanumericCaseInsensitive.ts **Line:** 30:33 **Comment:** *Logic Error: The use of a generic falsy check (`!valueA` / `!valueB`) treats empty strings and zero as "missing" values and always sorts them to the beginning or end, which is inconsistent with how valid values should be ordered and can surprise callers relying on proper string sorting. Replace the falsy checks with explicit null/undefined checks so that valid but falsy values like empty strings are still compared normally. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=9e4e33b23c5c4e19ce6dfd2aa8bcfdb1c46609f7d99f840b60cc566d87e15c7d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=9e4e33b23c5c4e19ce6dfd2aa8bcfdb1c46609f7d99f840b60cc566d87e15c7d&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/types/react-table.d.ts: ########## @@ -0,0 +1,117 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Merge typing interfaces for UseTable hooks. + * + * Ref: https://gist.github.com/ggascoigne/646e14c9d54258e40588a13aabf0102d + */ +import { + UseGlobalFiltersState, + UseGlobalFiltersOptions, + UseGlobalFiltersInstanceProps, + UsePaginationInstanceProps, + UsePaginationOptions, + UsePaginationState, + UseSortByColumnOptions, + UseSortByColumnProps, + UseSortByInstanceProps, + UseSortByOptions, + UseSortByState, + UseTableHooks, + UseSortByHooks, + UseColumnOrderState, + UseColumnOrderInstanceProps, + Renderer, + HeaderProps, + TableFooterProps, +} from 'react-table'; +import { DragEvent } from 'react'; Review Comment: **Suggestion:** The types `CSSProperties` and `MouseEventHandler` are referenced in the declaration of the toggle props but are never imported, which will cause TypeScript to report them as unknown types; they should be explicitly imported from `react` alongside `DragEvent`. [type error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Frontend TypeScript build fails on missing React type names. - ❌ CI type-check step blocks merging this plugin change. ``` </details> ```suggestion import { CSSProperties, DragEvent, MouseEventHandler } from 'react'; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open `superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/types/react-table.d.ts`. 2. Observe that `TableSortByToggleProps` is declared at lines ~83-88 using `CSSProperties` and `MouseEventHandler` types without qualification: `style?: CSSProperties;` and `onClick?: MouseEventHandler;`. 3. Note that this file only imports `DragEvent` from React at line 45 and does not import `CSSProperties` or `MouseEventHandler`. 4. Run the TypeScript compiler for the frontend workspace (which loads all `.d.ts` files under `superset-frontend` including this one); the compiler reports `Cannot find name 'CSSProperties'` and `Cannot find name 'MouseEventHandler'` for the usages in `TableSortByToggleProps`, because these types are not in the global namespace and are not imported in this declaration file. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/types/react-table.d.ts **Line:** 45:45 **Comment:** *Type Error: The types `CSSProperties` and `MouseEventHandler` are referenced in the declaration of the toggle props but are never imported, which will cause TypeScript to report them as unknown types; they should be explicitly imported from `react` alongside `DragEvent`. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=b0b4c60446921eda6a455168d726815852935c0da44aa2d786f681495b760260&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=b0b4c60446921eda6a455168d726815852935c0da44aa2d786f681495b760260&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/Styles.tsx: ########## @@ -0,0 +1,672 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { css, styled } from '@apache-superset/core/ui'; + +export default styled.div` + ${({ theme }) => css` + /* Base table styles */ + table { + width: 100%; + min-width: auto; + max-width: none; + margin: 0; + border-collapse: collapse; + } + + /* Context menu */ + .dt-context-menu { + position: fixed; + z-index: 10000; + background: ${theme.colorBgBase}; + border: 1px solid ${theme.colorBorderSecondary}; + box-shadow: none; + border-radius: ${theme.borderRadius}px; + padding: ${theme.paddingXS}px 0; + min-width: 180px; + } + .dt-context-menu .item { + padding: 6px 12px; + cursor: pointer; + color: ${theme.colorText}; + display: flex; + justify-content: space-between; + align-items: center; + gap: ${theme.marginXXS}px; + } + .dt-context-menu .item:hover { + background: ${theme.colorBgLayout}; + } + .dt-context-menu .separator { + height: 1px; + background: ${theme.colorSplit}; + margin: 4px 0; + } + + th, + td { + min-width: 4.3em; + padding: 0.75rem; + vertical-align: top; + } + + thead > tr > th { + padding-right: 0; + position: relative; + background-color: ${theme.colorBgBase}; + text-align: left; + border-bottom: 2px solid ${theme.colorSplit}; + color: ${theme.colorText}; + vertical-align: bottom; + } + /* Subtle actions header: no bottom border and muted color */ + thead > tr > th[data-column-name='actions'] { + border-bottom: none; + color: ${theme.colorTextTertiary}; + } + /* Make header ellipsis decorative (no hover/click) */ + thead > tr > th[data-column-name='actions'] .dt-ellipsis-button { + pointer-events: none; + cursor: default; + background: transparent; + border-color: transparent; + color: ${theme.colorTextTertiary}; + } + thead > tr > th[data-column-name='actions'] .dt-ellipsis-button:hover, + thead > tr > th[data-column-name='actions'] .dt-ellipsis-button:focus { + background: transparent; + border-color: transparent; + color: ${theme.colorTextTertiary}; + } + /* Column resize handle */ + .dt-col-resizer { + position: absolute; + top: 0; + right: 0; + width: 6px; + height: 100%; + cursor: col-resize; + user-select: none; + touch-action: none; + /* subtle visual cue on hover */ + } + .dt-col-resizer:hover { + background: ${theme.colorBgLayout}; + } + th svg { + margin: ${theme.sizeUnit / 2}px; + fill-opacity: 0.2; + } + th.is-sorted svg { + color: ${theme.colorText}; + fill-opacity: 1; + } + .table > tbody > tr:first-of-type > td, + .table > tbody > tr:first-of-type > th { + border-top: 0; + } + + .table > tbody tr td { + font-feature-settings: 'tnum' 1; + border-top: 1px solid ${theme.colorSplit}; + } + + /* Bootstrap-like condensed table styles */ + table.table-condensed, + table.table-sm { + font-size: ${theme.fontSizeSM}px; + } + + table.table-condensed th, + table.table-condensed td, + table.table-sm th, + table.table-sm td { + padding: 0.3rem; + } + + /* Bootstrap-like bordered table styles */ + table.table-bordered { + border: 1px solid ${theme.colorSplit}; + } + + table.table-bordered th, + table.table-bordered td { + border: 1px solid ${theme.colorSplit}; + } + + /* Bootstrap-like striped table styles */ + table.table-striped tbody tr:nth-of-type(odd) { + background-color: ${theme.colorBgLayout}; + } + + .dt-controls { + padding-bottom: 0.65em; + } + .dt-description { + /* top and bottom spacing using theme + slight indent */ + margin-block-start: ${theme.marginSM}px; + margin-block-end: ${theme.marginSM}px; + margin-inline-start: ${theme.marginXXS}px; /* RTL-aware indent */ + color: ${theme.colorText}; + /* ensure long content displays without overlap */ + word-break: break-word; + } + .dt-description img { + max-width: 100%; + height: auto; + } + .dt-description pre { + overflow: auto; + white-space: pre; + } + .dt-metric { + text-align: right; + } + .dt-totals { + font-weight: ${theme.fontWeightStrong}; + } + .dt-is-null { + color: ${theme.colorTextTertiary}; + } + td.dt-is-filter { + cursor: pointer; + } + td.dt-is-filter:hover { + background-color: ${theme.colorPrimaryBgHover}; + } + td.dt-is-active-filter, + td.dt-is-active-filter:hover { + background-color: ${theme.colorPrimaryBgHover}; + } + + .dt-global-filter { + float: right; + } + + .dt-truncate-cell { + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + } + .dt-truncate-cell:hover { + overflow: visible; + white-space: normal; + height: auto; + } + + .dt-pagination { + text-align: right; + /* use padding instead of margin so clientHeight can capture it */ + padding: ${theme.paddingXXS}px 0px; + } + .dt-pagination .pagination { + margin: 0; + padding-left: 0; + list-style: none; + display: inline-block; + white-space: nowrap; + } + + /* Align pagination item layout and spacing */ + .dt-pagination .pagination > li { + display: inline; + margin: 0 ${theme.marginXXS}px; + } + + /* Button look-and-feel to match core table */ + .dt-pagination .pagination > li > a, + .dt-pagination .pagination > li > span, + .dt-pagination .pagination > li > button { + background-color: ${theme.colorBgBase}; + color: ${theme.colorText}; + border: 1px solid transparent; /* no visible border for inactive */ + padding: ${theme.paddingXXS}px ${theme.paddingXS}px; + border-radius: ${theme.borderRadius}px; + display: inline-block; + text-decoration: none; + line-height: 1.2; + } + + /* Disabled pagination buttons */ + .dt-pagination .pagination > li > button[disabled] { + cursor: default; + opacity: 0.6; + } + + .dt-pagination .pagination > li > a:hover, + .dt-pagination .pagination > li > a:focus, + .dt-pagination .pagination > li > span:hover, + .dt-pagination .pagination > li > span:focus, + .dt-pagination .pagination > li > button:hover, + .dt-pagination .pagination > li > button:focus { + background: ${theme.colorBgLayout}; + border-color: transparent; /* keep border hidden on hover for inactive */ + color: ${theme.colorText}; + } + + /* Prevent hover effect on disabled */ + .dt-pagination .pagination > li > button[disabled]:hover, + .dt-pagination .pagination > li > button[disabled]:focus { + background: ${theme.colorBgBase}; + color: ${theme.colorText}; + } + + .dt-pagination .pagination.pagination-sm > li > a, + .dt-pagination .pagination.pagination-sm > li > span, + .dt-pagination .pagination.pagination-sm > li > button { + font-size: ${theme.fontSizeSM}px; + padding: ${theme.paddingXXS}px ${theme.paddingXS}px; + } + + /* Active page styles */ + .dt-pagination .pagination > li.active > a, + .dt-pagination .pagination > li.active > span, + .dt-pagination .pagination > li.active > button, + .dt-pagination .pagination > li.active > a:focus, + .dt-pagination .pagination > li.active > a:hover, + .dt-pagination .pagination > li.active > span:focus, + .dt-pagination .pagination > li.active > span:hover, + .dt-pagination .pagination > li.active > button:focus, + .dt-pagination .pagination > li.active > button:hover { + background-color: ${theme.colorPrimary}; + color: ${theme.colorBgContainer}; + border-color: ${theme.colorBorderSecondary}; + } + + /* Ellipsis item hover/focus */ + .pagination > li > span.dt-pagination-ellipsis:focus, + .pagination > li > span.dt-pagination-ellipsis:hover, + .dt-pagination .pagination > li.dt-pagination-ellipsis > span:focus, + .dt-pagination .pagination > li.dt-pagination-ellipsis > span:hover { + background: ${theme.colorBgLayout}; + border-color: ${theme.colorBorderSecondary}; + } + + /* Ellipsis default appearance */ + .dt-pagination .pagination > li.dt-pagination-ellipsis > span { + background: transparent; + border: 1px solid transparent; + color: ${theme.colorTextTertiary}; + cursor: default; + } + + .dt-no-results { + text-align: center; + padding: 1em 0.6em; + min-height: 48px; + } + + .right-border-only { border-right: 2px solid ${theme.colorSplit}; } + table .right-border-only:last-child { + border-right: none; + } + .selection-cell { + display: flex; + gap: 0.5rem; + align-items: center; + width: 3rem; + min-width: 3rem; + } + .selection-cell-number { + display: block; + text-overflow: ellipsis; + } + + /* Generic ellipsis trigger styling to match header/pagination ellipsis */ + .dt-ellipsis-button { + display: inline-flex; + align-items: center; + justify-content: center; + padding: 2px 4px; + min-width: 12px; + min-height: 12px; + border-radius: ${theme.borderRadius}px; + background: transparent; + border: 1px solid transparent; + color: #1a1a1a; /* near black for column menu icon */ + cursor: pointer; + line-height: 1; + user-select: none; + } + .dt-ellipsis-button:hover, + .dt-ellipsis-button:focus { + background: ${theme.colorBgLayout}; + border-color: ${theme.colorBorderSecondary}; + color: #000000; /* black on hover/focus */ + outline: none; + } + .dt-ellipsis-button svg { + fill-opacity: 1; /* override th svg opacity */ + } + .dt-tag-icon { + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; + margin-left: ${theme.marginXXS}px; + color: inherit; + } + .dt-tag-sep { + display: inline-flex; + align-items: center; + justify-content: center; + margin: 0 ${theme.marginXXS}px; + color: ${theme.colorTextTertiary}; + user-select: none; + } + + /* Advanced filter popover in header */ + .dt-filter-icon { + margin-left: 0; + color: #1a1a1a; /* near black for advanced filter icon */ + cursor: pointer; + display: inline-flex; + align-items: center; + justify-content: center; + padding: 2px 4px; + min-width: 12px; + min-height: 12px; + border-radius: ${theme.borderRadius}px; + background: transparent; + border: 1px solid transparent; + } + .dt-filter-icon:hover, + .dt-filter-icon:focus { + background: ${theme.colorBgLayout}; + border-color: ${theme.colorBorderSecondary}; + color: #000000; /* black on hover/focus */ + outline: none; + } + .dt-filter-icon.active { + color: #1a1a1a; /* keep near black when active */ + } + .dt-filter-icon.active:hover, + .dt-filter-icon.active:focus { + color: #000000; /* black on hover when active */ Review Comment: **Suggestion:** Icon styles for the ellipsis and advanced filter controls hardcode black hex colors instead of using theme tokens, which will make these icons effectively invisible or very low contrast in dark themes or customized themes, breaking readability and theming consistency. Replacing the hardcoded `#1a1a1a`/`#000000` with `${theme.colorText}` (or another appropriate theme token) keeps the icons legible across all theme variants. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Column menu ellipsis icons low-contrast in dark themes. - ⚠️ Advanced filter icons hard to see on dark backgrounds. - ⚠️ Theming consistency broken for this table plugin UI. ``` </details> ```suggestion color: ${theme.colorText}; cursor: pointer; line-height: 1; user-select: none; } .dt-ellipsis-button:hover, .dt-ellipsis-button:focus { background: ${theme.colorBgLayout}; border-color: ${theme.colorBorderSecondary}; color: ${theme.colorText}; outline: none; } .dt-ellipsis-button svg { fill-opacity: 1; /* override th svg opacity */ } .dt-tag-icon { display: inline-flex; align-items: center; justify-content: center; cursor: pointer; margin-left: ${theme.marginXXS}px; color: inherit; } .dt-tag-sep { display: inline-flex; align-items: center; justify-content: center; margin: 0 ${theme.marginXXS}px; color: ${theme.colorTextTertiary}; user-select: none; } /* Advanced filter popover in header */ .dt-filter-icon { margin-left: 0; color: ${theme.colorText}; cursor: pointer; display: inline-flex; align-items: center; justify-content: center; padding: 2px 4px; min-width: 12px; min-height: 12px; border-radius: ${theme.borderRadius}px; background: transparent; border: 1px solid transparent; } .dt-filter-icon:hover, .dt-filter-icon:focus { background: ${theme.colorBgLayout}; border-color: ${theme.colorBorderSecondary}; color: ${theme.colorText}; outline: none; } .dt-filter-icon.active { color: ${theme.colorText}; } .dt-filter-icon.active:hover, .dt-filter-icon.active:focus { color: ${theme.colorText}; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Start Superset with this PR code and enable a dark theme so `theme.colorBgBase` and `theme.colorBgLayout` are dark (see styled wrapper in `superset-frontend/plugins/plugin-chart-remita-table/src/Styles.tsx:1-23`). 2. Open any dashboard that uses the Remita table plugin so its root `<div>` styled by `Styles.tsx` is rendered and header ellipsis buttons get the `.dt-ellipsis-button` class defined at `Styles.tsx:331-347`. 3. Observe the ellipsis / column-menu icon color is hardcoded to `#1a1a1a`/`#000000` while the background is dark (`theme.colorBgBase`); contrast is much lower than the theme's `colorText`, making the control difficult to see in dark theme. 4. Open a column with an advanced filter and note the filter icon using `.dt-filter-icon` from `Styles.tsx:374-405` has the same hardcoded near-black colors, again ignoring `${theme.colorText}` and leading to inconsistent, low-contrast controls compared to other themed icons. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/Styles.tsx **Line:** 342:401 **Comment:** *Logic Error: Icon styles for the ellipsis and advanced filter controls hardcode black hex colors instead of using theme tokens, which will make these icons effectively invisible or very low contrast in dark themes or customized themes, breaking readability and theming consistency. Replacing the hardcoded `#1a1a1a`/`#000000` with `${theme.colorText}` (or another appropriate theme token) keeps the icons legible across all theme variants. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=94af36c648f861db56459e9e1b1ea67992e399cab02b219d843b78b5e45ca863&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=94af36c648f861db56459e9e1b1ea67992e399cab02b219d843b78b5e45ca863&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/useAsyncState.ts: ########## @@ -0,0 +1,51 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { useRef, useState } from 'react'; +import { useAsyncDebounce } from 'react-table'; + +// useAsyncDebounce in dist build of `react-table` requires regeneratorRuntime +import 'regenerator-runtime/runtime'; + +/** + * Hook useState to allow always return latest initialValue + */ +export default function useAsyncState<T, F extends (newValue: T) => unknown>( + initialValue: T, + callback: F, + wait = 200, +) { + const [value, setValue] = useState(initialValue); + const valueRef = useRef(initialValue); + const onChange = useAsyncDebounce(callback, wait); + + // sync updated initialValue + if (valueRef.current !== initialValue) { + valueRef.current = initialValue; + if (value !== initialValue) { + setValue(initialValue); + } + } + + const setBoth = (newValue: T) => { + setValue(newValue); + onChange(newValue); + }; + + return [value, setBoth] as [typeof value, typeof setValue]; Review Comment: **Suggestion:** The hook's returned setter is typed as React's `setState` (accepting both a value and an updater function) via `typeof setValue`, but the actual implementation only supports direct values and will pass any function argument through to the debounced callback, causing the callback to receive a function instead of the updated value and leading to subtle runtime logic errors when consumers use functional updates. [type error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Remita table hook misbehaves if used with functional updates. - ⚠️ Setter typing misleads developers, enabling subtle future runtime bugs. ``` </details> ```suggestion return [value, setBoth] as [T, (newValue: T) => void]; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. In any React component, import `useAsyncState` from `superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/useAsyncState.ts:28` and declare `const [value, setValue] = useAsyncState<number, (v: number) => void>(0, callbackFn);`. 2. Rely on the inferred setter type from the hook's return at `useAsyncState.ts:50`, which is `typeof setValue` from React, and call `setValue(prev => prev + 1)` using a functional update. 3. At runtime, `setBoth` (defined at `useAsyncState.ts:45`) receives the function `prev => prev + 1` as `newValue` (since its signature is `(newValue: T)`), stores that function into React state via `setValue(newValue)`, and passes the same function to the debounced `onChange(newValue)` callback. 4. The consumer callback `callbackFn` (passed into `useAsyncState` at `useAsyncState.ts:30`) is now invoked with the function object instead of the expected numeric value, causing incorrect logic or type errors in any real usage that assumes `newValue` is a number; this mismatch is only possible because the returned setter type incorrectly advertises functional updates despite the implementation not supporting them. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/useAsyncState.ts **Line:** 50:50 **Comment:** *Type Error: The hook's returned setter is typed as React's `setState` (accepting both a value and an updater function) via `typeof setValue`, but the actual implementation only supports direct values and will pass any function argument through to the debounced callback, causing the callback to receive a function instead of the updated value and leading to subtle runtime logic errors when consumers use functional updates. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=571a5c37de16db7d09eee01d367401885e550b482d67e549ea3494f16a4aaf0f&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=571a5c37de16db7d09eee01d367401885e550b482d67e549ea3494f16a4aaf0f&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/hooks/useSticky.tsx: ########## @@ -0,0 +1,451 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { + Children, + cloneElement, + useRef, + useMemo, + useLayoutEffect, + useCallback, + ReactNode, + ReactElement, + ComponentPropsWithRef, + CSSProperties, + UIEventHandler, +} from 'react'; +import { TableInstance, Hooks } from 'react-table'; +import getScrollBarSize from '../utils/getScrollBarSize'; +import needScrollBar from '../utils/needScrollBar'; +import useMountedMemo from '../utils/useMountedMemo'; + +type ReactElementWithChildren< + T extends keyof JSX.IntrinsicElements, + C extends ReactNode = ReactNode, +> = ReactElement<ComponentPropsWithRef<T> & { children: C }, T>; + +type Th = ReactElementWithChildren<'th'>; +type Td = ReactElementWithChildren<'td'>; +type TrWithTh = ReactElementWithChildren<'tr', Th[]>; +type TrWithTd = ReactElementWithChildren<'tr', Td[]>; +type Thead = ReactElementWithChildren<'thead', TrWithTh>; +type Tbody = ReactElementWithChildren<'tbody', TrWithTd>; +type Tfoot = ReactElementWithChildren<'tfoot', TrWithTd>; +type Col = ReactElementWithChildren<'col', null>; +type ColGroup = ReactElementWithChildren<'colgroup', Col>; + +export type Table = ReactElementWithChildren< + 'table', + (Thead | Tbody | Tfoot | ColGroup)[] +>; +export type TableRenderer = () => Table; +export type GetTableSize = () => Partial<StickyState> | undefined; +export type SetStickyState = (size?: Partial<StickyState>) => void; + +export enum ReducerActions { + Init = 'init', // this is from global reducer + SetStickyState = 'setStickyState', +} + +export type ReducerAction< + T extends string, + P extends Record<string, unknown>, +> = P & { type: T }; + +export type ColumnWidths = number[]; + +export interface StickyState { + width?: number; // maximum full table width + height?: number; // maximum full table height + realHeight?: number; // actual table viewport height (header + scrollable area) + bodyHeight?: number; // scrollable area height + tableHeight?: number; // the full table height + columnWidths?: ColumnWidths; + hasHorizontalScroll?: boolean; + hasVerticalScroll?: boolean; + rendering?: boolean; + setStickyState?: SetStickyState; +} + +export interface UseStickyTableOptions { + getTableSize?: GetTableSize; +} + +export interface UseStickyInstanceProps { + // manipulate DOMs in <table> to make the header sticky + wrapStickyTable: (renderer: TableRenderer) => ReactNode; + // update or recompute the sticky table size + setStickyState: SetStickyState; +} + +export type UseStickyState = { + sticky: StickyState; +}; + +const sum = (a: number, b: number) => a + b; +const mergeStyleProp = ( + node: ReactElement<{ style?: CSSProperties }>, + style: CSSProperties, +) => ({ + style: { + ...node.props.style, + ...style, + }, +}); +const fixedTableLayout: CSSProperties = { tableLayout: 'fixed' }; + +/** + * An HOC for generating sticky header and fixed-height scrollable area + */ +function StickyWrap({ + sticky = {}, + width: maxWidth, + height: maxHeight, + children: table, + setStickyState, +}: { + width: number; + height: number; + setStickyState: SetStickyState; + children: Table; + sticky?: StickyState; // current sticky element sizes +}) { + if (!table || table.type !== 'table') { + throw new Error('<StickyWrap> must have only one <table> element as child'); + } + let thead: Thead | undefined; + let tbody: Tbody | undefined; + let tfoot: Tfoot | undefined; + + Children.forEach(table.props.children, node => { + if (!node) { + return; + } + if (node.type === 'thead') { + thead = node; + } else if (node.type === 'tbody') { + tbody = node; + } else if (node.type === 'tfoot') { + tfoot = node; + } + }); + if (!thead || !tbody) { + throw new Error( + '<table> in <StickyWrap> must contain both thead and tbody.', + ); + } + const columnCount = useMemo(() => { + const headerRows = Children.toArray( + thead?.props.children, + ).pop() as TrWithTh; + return headerRows.props.children.length; + }, [thead]); + + const theadRef = useRef<HTMLTableSectionElement>(null); // original thead for layout computation + const tfootRef = useRef<HTMLTableSectionElement>(null); // original tfoot for layout computation + const scrollHeaderRef = useRef<HTMLDivElement>(null); // fixed header + const scrollFooterRef = useRef<HTMLDivElement>(null); // fixed footer + const scrollBodyRef = useRef<HTMLDivElement>(null); // main body + + const scrollBarSize = getScrollBarSize(); + const { bodyHeight, columnWidths } = sticky; + const needSizer = + !columnWidths || + sticky.width !== maxWidth || + sticky.height !== maxHeight || + sticky.setStickyState !== setStickyState; + + // update scrollable area and header column sizes when mounted + useLayoutEffect(() => { + if (!theadRef.current) { + return; + } + let raf = 0; + const measure = () => { + const bodyThead = theadRef.current!; + const theadHeight = bodyThead.clientHeight; + const tfootHeight = tfootRef.current ? tfootRef.current.clientHeight : 0; + if (!theadHeight) { + return; + } + const fullTableHeight = (bodyThead.parentNode as HTMLTableElement) + .clientHeight; + // instead of always using the first tr, we use the last one to support + // multi-level headers assuming the last one is the more detailed one + const ths = bodyThead.childNodes?.[bodyThead.childNodes?.length - 1 || 0] + .childNodes as NodeListOf<HTMLTableHeaderCellElement>; + const widths = Array.from(ths).map( + th => th.getBoundingClientRect()?.width || th.clientWidth, + ); + const [hasVerticalScroll, hasHorizontalScroll] = needScrollBar({ + width: maxWidth, + height: maxHeight - theadHeight - tfootHeight, + innerHeight: fullTableHeight, + innerWidth: widths.reduce(sum), Review Comment: **Suggestion:** Reducing `widths` without an initial value will throw a runtime error when there are no header cells (empty `ths`), so the scroll size computation will crash instead of treating the total width as zero. [possible bug] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Sticky header measurement crashes for malformed header DOM. - ⚠️ Remita table chart fails when header has zero cells. ``` </details> ```suggestion const totalWidth = widths.reduce(sum, 0); const [hasVerticalScroll, hasHorizontalScroll] = needScrollBar({ width: maxWidth, height: maxHeight - theadHeight - tfootHeight, innerHeight: fullTableHeight, innerWidth: totalWidth, ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. A React table using the sticky header hook mounts and renders `<StickyWrap>` in `useSticky.tsx:114-157` via `wrapStickyTable` returned from `useInstance` (`useSticky.tsx:360-417`), with a `<thead>` that contains at least one `<tr>` but no `<th>` cells in the final DOM (so `ths.length === 0` in the effect). 2. On mount, the `useLayoutEffect` in `StickyWrap` (`useSticky.tsx:173-228`) runs and `theadRef.current` points at the rendered `<thead>`; `bodyThead.childNodes[bodyThead.childNodes.length - 1].childNodes` (lines 190-191) returns an empty `NodeListOf<HTMLTableHeaderCellElement>`. 3. The code at `useSticky.tsx:192-194` builds `widths = Array.from(ths).map(...)`, resulting in `widths` being an empty array (`[]`) because there are no header cells. 4. At `useSticky.tsx:195-201`, `widths.reduce(sum)` is executed without an initial value, which throws `TypeError: Reduce of empty array with no initial value` at runtime, breaking sticky table measurement and preventing the Remita table chart from rendering correctly. In normal Superset table usage, headers are always populated via react-table column definitions, so constructing a header row with zero cells is an invalid/misconfigured usage and makes this issue a low-occurrence edge case. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/hooks/useSticky.tsx **Line:** 195:199 **Comment:** *Possible Bug: Reducing `widths` without an initial value will throw a runtime error when there are no header cells (empty `ths`), so the scroll size computation will crash instead of treating the total width as zero. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=7d38731094c1a993259b95212c55691a2b9708ae82db1b3e3daca3a575fc7fd2&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=7d38731094c1a993259b95212c55691a2b9708ae82db1b3e3daca3a575fc7fd2&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/needScrollBar.ts: ########## @@ -0,0 +1,39 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +/** + * Whether a container need scroll bars when in another container. + */ +export default function needScrollBar({ + width, + height, + innerHeight, + innerWidth, + scrollBarSize, +}: { + width: number; + height: number; + innerHeight: number; + scrollBarSize: number; + innerWidth: number; +}): [boolean, boolean] { + const hasVerticalScroll = innerHeight > height; + const hasHorizontalScroll = + innerWidth > width - (hasVerticalScroll ? scrollBarSize : 0); Review Comment: **Suggestion:** The vertical scrollbar calculation does not account for the height reduction caused by a horizontal scrollbar, so in cases where content width alone forces a horizontal scrollbar, the effective height shrinks but `hasVerticalScroll` remains false, leading to an incorrect result (horizontal-only when both scrollbars will actually appear). Re-evaluating the vertical scrollbar when a horizontal scrollbar is needed fixes this logic error. [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Remita table DataTable may mis-detect vertical scrolling needs. - ⚠️ Layout logic relying on needScrollBar receives incorrect flags. ``` </details> ```suggestion let hasVerticalScroll = innerHeight > height; let hasHorizontalScroll = innerWidth > width - (hasVerticalScroll ? scrollBarSize : 0); if (hasHorizontalScroll && !hasVerticalScroll) { hasVerticalScroll = innerHeight > height - scrollBarSize; } ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Open `superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/needScrollBar.ts` and locate the default-exported function `needScrollBar` at lines 24–38, which returns a tuple `[hasVerticalScroll, hasHorizontalScroll]` based on `width`, `height`, `innerWidth`, `innerHeight`, and `scrollBarSize`. 2. From any caller (or a unit test), invoke `needScrollBar` with parameters that force only horizontal scroll in the current logic but would force both scrollbars in a real container, e.g.: - `width = 100`, `height = 100` - `innerWidth = 120` (content wider than container) - `innerHeight = 100` (content exactly equal to container height) - `scrollBarSize = 15` Example call: `needScrollBar({ width: 100, height: 100, innerWidth: 120, innerHeight: 100, scrollBarSize: 15 });` 3. Observe the current implementation at lines 35–38: - `hasVerticalScroll = innerHeight > height` → `100 > 100` → `false` - `hasHorizontalScroll = innerWidth > width - (hasVerticalScroll ? scrollBarSize : 0)` → `120 > 100 - 0` → `true` - Function returns `[false, true]`, i.e. "no vertical scroll, horizontal scroll present". 4. In a real browser layout, a horizontal scrollbar of `scrollBarSize = 15` reduces the effective visible height from 100px to 85px, so the same `innerHeight = 100` would require a vertical scrollbar (since `100 > 85`). Any DataTable code using the `[false, true]` result from `needScrollBar` (file `needScrollBar.ts`) to size or style its scrollable container will treat the container as horizontally scrollable only, while the actual DOM will show both scrollbars, leading to incorrect layout assumptions (e.g., misalignment or unexpected overflow). ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/utils/needScrollBar.ts **Line:** 35:37 **Comment:** *Logic Error: The vertical scrollbar calculation does not account for the height reduction caused by a horizontal scrollbar, so in cases where content width alone forces a horizontal scrollbar, the effective height shrinks but `hasVerticalScroll` remains false, leading to an incorrect result (horizontal-only when both scrollbars will actually appear). Re-evaluating the vertical scrollbar when a horizontal scrollbar is needed fixes this logic error. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=1b9683f34659f7cdf77946996d78d8c9f2016a496717b5554a646df5e9ae3116&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=1b9683f34659f7cdf77946996d78d8c9f2016a496717b5554a646df5e9ae3116&reaction=dislike'>👎</a> ########## superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/DataTable.tsx: ########## @@ -0,0 +1,851 @@ +import { CSSProperties, DragEvent, HTMLProps, MutableRefObject, ReactNode, useCallback, useEffect, useRef, useState, cloneElement, isValidElement, Children, useMemo } from 'react'; + +import { + FilterType, + IdType, + PluginHook, + Row, + TableOptions, + useColumnOrder, + useGlobalFilter, + usePagination, + useSortBy, + useTable, +} from 'react-table'; +import {matchSorter, rankings} from 'match-sorter'; +import {typedMemo, usePrevious} from '@superset-ui/core'; +import {isEqual} from 'lodash'; +import GlobalFilter, {GlobalFilterProps} from './components/GlobalFilter'; +import SelectPageSize, {SelectPageSizeProps, SizeOption,} from './components/SelectPageSize'; +import SimplePagination from './components/Pagination'; +import SearchSelectDropdown from './components/SearchSelectDropdown'; +import useSticky from './hooks/useSticky'; +import {PAGE_SIZE_OPTIONS} from '../consts'; +import {sortAlphanumericCaseInsensitive} from './utils/sortAlphanumericCaseInsensitive'; +import {BulkActions} from '../BulkActions'; + +export interface BulkAction { + key: string; + label: string; + style?: any + boundToSelection: boolean; + visibilityCondition: 'all' | 'selected' | 'unselected'; + showInSliceHeader: boolean; + value?: string; + rowId?: string; + type?: any; +} + +export interface BulkActionsConfig { + split: Set<any>; + nonSplit: Set<any>; + value?: string; + rowId?: string; + type?: any; +} + +export interface DataTableProps<D extends object> extends TableOptions<D> { + tableClassName?: string; + searchInput?: boolean | GlobalFilterProps<D>['searchInput']; + selectPageSize?: boolean | SelectPageSizeProps['selectRenderer']; + pageSizeOptions?: SizeOption[]; // available page size options + maxPageItemCount?: number; + hooks?: PluginHook<D>[]; // any additional hooks + width?: string | number; + height?: string | number; + serverPagination?: boolean; + onServerPaginationChange: (pageNumber: number, pageSize: number) => void; + serverPaginationData: { pageSize?: number; currentPage?: number; searchColumn?: string; searchText?: string; sortBy?: any[] }; + pageSize?: number; + noResults?: string | ((filterString: string) => ReactNode); + sticky?: boolean; + rowCount: number; + wrapperRef?: MutableRefObject<HTMLDivElement>; + onColumnOrderChange: () => void; + renderGroupingHeaders?: () => JSX.Element; + renderTimeComparisonDropdown?: () => JSX.Element; + selectedRows?: Map<string, D>; + bulkActions?: BulkActionsConfig; + onBulkActionClick?: (actionKey: string, selectedIds: any[]) => void; + onClearSelection?: () => void; + // Server-side search/sort support (like core table) + manualSearch?: boolean; + onSearchChange?: (query: string) => void; + initialSearchText?: string; + sortByFromParent?: any[]; + handleSortByChange?: (sortBy: any[]) => void; + searchOptions?: { value: string; label: string }[]; + onSearchColChange?: (searchCol: string) => void; + enableBulkActions?: boolean; + includeRowNumber?: boolean; + enableTableActions?: boolean; + tableActionsIdColumn?: string; + hideTableActionsIdColumn?: boolean; + bulkActionLabel?: string; + tableActions?: Set<any>; + onTableActionClick?: (action?: string, id?: string, value?: string) => void; + showSplitInSliceHeader?: boolean; + retainSelectionAccrossNavigation?: boolean; + chartId?: string; + showSearchColumnSelector?: boolean; + onInvertSelection?: () => void; + enableColumnResize?: boolean; + // optional trigger element to render in top controls (e.g., advanced filters toggle) + renderAdvancedFiltersTrigger?: () => JSX.Element; + // optional right-side controls next to search input + renderRightControls?: () => JSX.Element; +} + +export interface RenderHTMLCellProps extends HTMLProps<HTMLTableCellElement> { + cellContent: ReactNode; +} + +const sortTypes = { + alphanumeric: sortAlphanumericCaseInsensitive, +}; + +// Be sure to pass our updateMyData and the skipReset option +export default typedMemo(function DataTable<D extends object>({ + tableClassName, + columns, + data, + serverPaginationData, + width: initialWidth = '100%', + height: initialHeight = 300, + pageSize: initialPageSize = 0, + initialState: initialState_ = {}, + pageSizeOptions = PAGE_SIZE_OPTIONS, + maxPageItemCount = 9, + sticky: doSticky, + searchInput = true, + onServerPaginationChange, + rowCount, + selectPageSize, + noResults: noResultsText = 'No data found', + hooks, + serverPagination, + wrapperRef: userWrapperRef, + onColumnOrderChange, + renderGroupingHeaders, + renderTimeComparisonDropdown, + selectedRows, + bulkActionLabel, + manualSearch, + onSearchChange, + initialSearchText, + sortByFromParent, + handleSortByChange, + searchOptions, + onSearchColChange, + bulkActions, + onBulkActionClick, + onClearSelection, + tableActionsIdColumn, + hideTableActionsIdColumn = false, + enableBulkActions = false, + showSplitInSliceHeader = false, + chartId, + showSearchColumnSelector, + onInvertSelection, + enableColumnResize, + renderAdvancedFiltersTrigger, + renderRightControls, + ...moreUseTableOptions + }: DataTableProps<D>): JSX.Element { + const tableHooks: PluginHook<D>[] = [ + useGlobalFilter, + useSortBy, + usePagination, + useColumnOrder, + doSticky ? useSticky : [], + hooks || [], + ].flat(); + + const columnNames = Object.keys(data?.[0] || {}); + + const previousColumnNames = usePrevious(columnNames); + + const resultsSize = serverPagination ? rowCount : data.length; + const sortByRef = useRef([]); // cache initial `sortby` so sorting doesn't trigger page reset + // Derive the desired page size for the table (react-table) to display + const serverPageSizeForTable = serverPagination + ? (serverPaginationData?.pageSize ?? initialPageSize) + : undefined; + const desiredPageSize = serverPagination + ? (serverPageSizeForTable !== undefined && serverPageSizeForTable !== null ? serverPageSizeForTable : (initialPageSize !== undefined && initialPageSize !== null ? initialPageSize : 50)) + : (initialPageSize > 0 ? initialPageSize : resultsSize || 10); + const pageSizeRef = useRef([desiredPageSize, resultsSize]); + // For server pagination, always check if pagination needed based on server page size + // For client pagination, pageSize == 0 means no pagination + const hasPagination = serverPagination + ? (serverPageSizeForTable !== undefined && serverPageSizeForTable !== null && serverPageSizeForTable > 0 && resultsSize > 0) + : (initialPageSize > 0 && resultsSize > 0); + // Show controls if we have pagination, search, time comparison, or explicit page-size selector + const showPageSizeControl = Boolean(selectPageSize); + const hasGlobalControl = hasPagination || !!searchInput || renderTimeComparisonDropdown || showPageSizeControl; + + // Build initial column order from persisted column keys (if any) + let initialColumnOrder: string[] | undefined; + try { + const persisted = chartId ? localStorage.getItem(`columnOrder_${chartId}`) : null; + if (persisted) { + const savedKeys: string[] = JSON.parse(persisted); + const colIdByKey = new Map<string, string>(); + (columns as any[]).forEach(col => { + if (col && typeof col === 'object') { + const id = (col as any).id; + const key = (col as any).columnKey; + if (id != null && key != null) colIdByKey.set(String(key), String(id)); + } + }); + const idsFromSaved = savedKeys + .map(k => colIdByKey.get(String(k))) + .filter((v): v is string => Boolean(v)); + const existingIds = new Set(idsFromSaved); + const remainingIds = (columns as any[]) + .map(c => String((c as any).id)) + .filter(id => !existingIds.has(id)); + initialColumnOrder = [...idsFromSaved, ...remainingIds]; + } + } catch { + // ignore + } + + const initialState = { + ...initialState_, + // zero length means all pages, the `usePagination` plugin does not + // understand pageSize = 0 + // Respect incoming sort from parent (server-side) as initial state when provided + sortBy: (serverPagination && Array.isArray(sortByFromParent) && sortByFromParent.length) + ? (sortByFromParent as any) + : sortByRef.current, + pageIndex: serverPagination ? (serverPaginationData?.currentPage ?? 0) : (initialState_ as any)?.pageIndex ?? 0, + pageSize: desiredPageSize, + ...(initialColumnOrder ? { columnOrder: initialColumnOrder } : {}), + }; + + const defaultWrapperRef = useRef<HTMLDivElement>(null); + const globalControlRef = useRef<HTMLDivElement>(null); + const paginationRef = useRef<HTMLDivElement>(null); + const wrapperRef = userWrapperRef || defaultWrapperRef; + // For height calculations, only page number and size matter; avoid JSON.stringify + const paginationKey = serverPagination + ? `${serverPaginationData?.currentPage ?? 0}:${serverPaginationData?.pageSize ?? initialPageSize}` + : ''; + + // Pre-compute pageCount for react-table when using server-side pagination + const serverPageSize = serverPagination + ? (serverPaginationData?.pageSize !== undefined && serverPaginationData?.pageSize !== null + ? serverPaginationData.pageSize + : (initialPageSize !== undefined && initialPageSize !== null + ? initialPageSize + : 50)) + : undefined; + const serverPageCount = serverPagination && serverPageSize + ? Math.ceil(rowCount / serverPageSize) + : undefined; + + // Cache container width once per render to avoid repeated layout reads + const containerWidthMemo = useMemo(() => { + try { + return Number(initialWidth) || (wrapperRef.current?.clientWidth || 0); + } catch { return 0; } + }, [initialWidth]); Review Comment: **Suggestion:** The computed `containerWidthMemo` only re-computes when `initialWidth` changes and ignores changes to `wrapperRef.current`, so when `initialWidth` is a non-numeric value like '100%' it will stay at 0 permanently, breaking any header/cell logic that depends on the actual container width (e.g., column resize). [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Remita table chart column resize uses container width 0. - ⚠️ Sticky header layout may misalign due to zero width. ``` </details> ```suggestion const containerWidthMemo = (() => { try { return Number(initialWidth) || (wrapperRef.current?.clientWidth || 0); } catch { return 0; } })(); ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Render the `DataTable` component from `superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/DataTable.tsx` without passing a numeric `width` prop (or omit `width` entirely). Due to the function signature at lines ~90–115 (`width: initialWidth = '100%'`), `initialWidth` will be the string `'100%'`. 2. On the initial render, before React assigns `ref`, the memoized `containerWidthMemo` is computed at lines 248–253: `const containerWidthMemo = useMemo(() => { ... }, [initialWidth]);`. At this moment `wrapperRef` (created at lines ~200–207 and attached to `<div ref={wrapperRef} ...>` at the bottom JSX) still has `wrapperRef.current === null`. 3. Inside the memo callback (lines 249–252), `Number(initialWidth)` with `initialWidth === '100%'` evaluates to `NaN` (falsy), so the expression falls back to `(wrapperRef.current?.clientWidth || 0)`. Because `wrapperRef.current` is `null` during this render, this expression evaluates to `0`, so `containerWidthMemo` is set to `0`. Since the `useMemo` dependency array is `[initialWidth]` and `initialWidth` remains `'100%'`, `containerWidthMemo` will never recompute, even after the ref is attached and the container has a non-zero `clientWidth`. 4. When the table header is rendered, at lines ~531–568 in `renderTable`, each column header is rendered via `column.render('Header', { ..., enableColumnResize, setStickyState, containerWidth: containerWidthMemo, ... })`. Any header implementation that relies on the `containerWidth` prop (e.g., for column resize or width calculations) will always receive `0`, causing width-dependent behaviors (such as proportional column sizing or resize handles) to be calculated against a zero-width container in every Remita table instance that uses the default `'100%'` width or a non-numeric string width. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/plugins/plugin-chart-remita-table/src/DataTable/DataTable.tsx **Line:** 249:253 **Comment:** *Logic Error: The computed `containerWidthMemo` only re-computes when `initialWidth` changes and ignores changes to `wrapperRef.current`, so when `initialWidth` is a non-numeric value like '100%' it will stay at 0 permanently, breaking any header/cell logic that depends on the actual container width (e.g., column resize). Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=34f50f3db09c965cbce0515f45dab9ec290aaf798baacf7eaad65505b35dc321&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37784&comment_hash=34f50f3db09c965cbce0515f45dab9ec290aaf798baacf7eaad65505b35dc321&reaction=dislike'>👎</a> -- 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]
