diff --git a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts index 66e2c513080..2e1e7f6c820 100644 --- a/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts +++ b/superset-frontend/cypress-base/cypress/e2e/explore/control.test.ts @@ -40,15 +40,11 @@ describe('Datasource control', () => { // create new metric cy.get('[data-test="crud-add-table-item"]', { timeout: 10000 }).click(); cy.wait(1000); - cy.get( - '[data-test="table-content-rows"] [data-test="textarea-editable-title-input"]', - ) + cy.get('.antd5-table-body [data-test="editable-title-input"]') .first() .click(); - cy.get( - '[data-test="table-content-rows"] [data-test="textarea-editable-title-input"]', - ) + cy.get('.antd5-table-body [data-test="editable-title-input"]') .first() .focus(); cy.focused().clear({ force: true }); diff --git a/superset-frontend/cypress-base/cypress/support/directories.ts b/superset-frontend/cypress-base/cypress/support/directories.ts index 1f1d5a543fc..05f6aecefbf 100644 --- a/superset-frontend/cypress-base/cypress/support/directories.ts +++ b/superset-frontend/cypress-base/cypress/support/directories.ts @@ -583,7 +583,7 @@ export const exploreView = { saveButton: dataTestLocator('datasource-modal-save'), metricsTab: { addItem: dataTestLocator('crud-add-table-item'), - rowsContainer: dataTestLocator('table-content-rows'), + rowsContainer: '.antd5-table-body', }, confirmModal: { okButton: '.antd5-modal-confirm-btns .antd5-btn-primary', diff --git a/superset-frontend/src/components/Datasource/CollectionTable.test.tsx b/superset-frontend/src/components/Datasource/CollectionTable.test.tsx index 1ebe7406d91..69c7fc09a0b 100644 --- a/superset-frontend/src/components/Datasource/CollectionTable.test.tsx +++ b/superset-frontend/src/components/Datasource/CollectionTable.test.tsx @@ -34,6 +34,6 @@ test('renders a table', () => { expect( getByRole('table') .getElementsByTagName('tbody')[0] - .getElementsByClassName('row'), - ).toHaveLength(length); + .getElementsByTagName('tr'), + ).toHaveLength(length + 1); // Ant design measure row; }); diff --git a/superset-frontend/src/components/Datasource/CollectionTable.tsx b/superset-frontend/src/components/Datasource/CollectionTable.tsx index 075be69a902..63bfb2ebf9d 100644 --- a/superset-frontend/src/components/Datasource/CollectionTable.tsx +++ b/superset-frontend/src/components/Datasource/CollectionTable.tsx @@ -16,87 +16,30 @@ * specific language governing permissions and limitations * under the License. */ -import { PureComponent } from 'react'; +import { PureComponent, ReactNode } from 'react'; import { nanoid } from 'nanoid'; import { InfoTooltipWithTrigger } from '@superset-ui/chart-controls'; -import { t, styled } from '@superset-ui/core'; +import { t, styled, css } from '@superset-ui/core'; import { Icons } from 'src/components/Icons'; +import { FilterValue } from 'react-table'; import { Button } from '../Button'; import Fieldset from './Fieldset'; import { recurseReactClone } from './utils'; import { - SortOrder, type CRUDCollectionProps, type CRUDCollectionState, type Sort, } from './types'; - -function createCollectionArray(collection: Record) { - return Object.keys(collection).map(k => collection[k]); -} - -function createKeyedCollection(arr: Array) { - const collectionArray = arr.map((o: any) => ({ - ...o, - id: o.id || nanoid(), - })); - - const collection: Record = {}; - collectionArray.forEach((o: any) => { - collection[o.id] = o; - }); - - return { - collection, - collectionArray, - }; -} - -const CrudTableWrapper = styled.div<{ stickyHeader?: boolean }>` - ${({ stickyHeader }) => - stickyHeader && - ` - height: 350px; - overflow-y: auto; - overflow-x: auto; - - .table { - min-width: 800px; - } - thead th { - background: #fff; - position: sticky; - top: 0; - z-index: 9; - min - } - `} - ${({ theme }) => ` - th span { - vertical-align: ${theme.sizeUnit * -2}px; - } - .text-right { - text-align: right; - } - .empty-collection { - padding: ${theme.sizeUnit * 2 + 2}px; - } - .tiny-cell { - width: ${theme.sizeUnit + 1}px; - } - i.fa-caret-down, - i.fa-caret-up { - width: ${theme.sizeUnit + 1}px; - } - td.expanded { - border-top: 0; - padding: 0; - } - `} -`; +import Table, { + type ColumnsType, + type SortOrder, + type SorterResult, + type TablePaginationConfig, + TableSize, +} from '../Table'; const CrudButtonWrapper = styled.div` text-align: right; @@ -113,6 +56,32 @@ const StyledButtonWrapper = styled.span` `} `; +type CollectionItem = { id: string | number; [key: string]: any }; + +function createCollectionArray(collection: Record) { + return Object.keys(collection).map(k => collection[k] as CollectionItem); +} + +function createKeyedCollection(arr: Array) { + const collectionArray = arr.map( + (o: any) => + ({ + ...o, + id: o.id || nanoid(), + }) as CollectionItem, + ); + + const collection: Record = {}; + collectionArray.forEach((o: CollectionItem) => { + collection[o.id] = o; + }); + + return { + collection, + collectionArray, + }; +} + export default class CRUDCollection extends PureComponent< CRUDCollectionProps, CRUDCollectionState @@ -130,15 +99,14 @@ export default class CRUDCollection extends PureComponent< sortColumn: '', sort: 0, }; - this.renderItem = this.renderItem.bind(this); this.onAddItem = this.onAddItem.bind(this); this.renderExpandableSection = this.renderExpandableSection.bind(this); this.getLabel = this.getLabel.bind(this); this.onFieldsetChange = this.onFieldsetChange.bind(this); - this.renderTableBody = this.renderTableBody.bind(this); this.changeCollection = this.changeCollection.bind(this); - this.sortColumn = this.sortColumn.bind(this); - this.renderSortIcon = this.renderSortIcon.bind(this); + this.handleTableChange = this.handleTableChange.bind(this); + this.buildTableColumns = this.buildTableColumns.bind(this); + this.toggleExpand = this.toggleExpand.bind(this); } UNSAFE_componentWillReceiveProps(nextProps: CRUDCollectionProps) { @@ -146,30 +114,69 @@ export default class CRUDCollection extends PureComponent< const { collection, collectionArray } = createKeyedCollection( nextProps.collection, ); - this.setState({ + this.setState(prevState => ({ collection, collectionArray, - }); + expandedColumns: prevState.expandedColumns, + })); } } onCellChange(id: number, col: string, val: boolean) { - this.changeCollection({ - ...this.state.collection, - [id]: { - ...this.state.collection[id], - [col]: val, - }, + this.setState(prevState => { + const updatedCollection = { + ...prevState.collection, + [id]: { + ...prevState.collection[id], + [col]: val, + }, + }; + const updatedCollectionArray = prevState.collectionArray.map(item => + item.id === id ? updatedCollection[id] : item, + ); + + if (this.props.onChange) { + this.props.onChange(updatedCollectionArray); + } + return { + collection: updatedCollection, + collectionArray: updatedCollectionArray, + }; }); } onAddItem() { if (this.props.itemGenerator) { let newItem = this.props.itemGenerator(); + const shouldStartExpanded = newItem.expanded === true; if (!newItem.id) { newItem = { ...newItem, id: nanoid() }; } - this.changeCollection(this.state.collection, newItem); + delete newItem.expanded; + + this.setState( + prevState => { + const newCollection = { + ...prevState.collection, + [newItem.id]: newItem, + }; + const newExpandedColumns = shouldStartExpanded + ? { ...prevState.expandedColumns, [newItem.id]: true } + : prevState.expandedColumns; + const newCollectionArray = [newItem, ...prevState.collectionArray]; + + return { + collection: newCollection, + collectionArray: newCollectionArray, + expandedColumns: newExpandedColumns, + }; + }, + () => { + if (this.props.onChange) { + this.props.onChange(this.state.collectionArray); + } + }, + ); } } @@ -180,52 +187,38 @@ export default class CRUDCollection extends PureComponent< }); } - getLabel(col: any) { + getLabel(col: any): string { const { columnLabels } = this.props; let label = columnLabels?.[col] ? columnLabels[col] : col; if (label.startsWith('__')) { - // special label-free columns (ie: caret for expand, delete cross) label = ''; } return label; } - getTooltip(col: string) { + getTooltip(col: string): string | undefined { const { columnLabelTooltips } = this.props; return columnLabelTooltips?.[col]; } - changeCollection(collection: any, newItem?: object) { - this.setState({ collection }); - if (this.props.onChange) { - const collectionArray = this.state.collectionArray - .map((c: { id: number }) => collection[c.id]) - // filter out removed items - .filter(c => c !== undefined); + changeCollection(collection: any) { + console.log({ collection }); + const newCollectionArray = createCollectionArray(collection); + console.log({ newCollectionArray }); + this.setState({ collection, collectionArray: newCollectionArray }); - if (newItem) { - collectionArray.unshift(newItem); - } - this.props.onChange(collectionArray); + if (this.props.onChange) { + this.props.onChange(newCollectionArray); } } - deleteItem(id: number) { + deleteItem(id: string | number) { const newColl = { ...this.state.collection }; delete newColl[id]; this.changeCollection(newColl); } - effectiveTableColumns() { - const { tableColumns, allowDeletes, expandFieldset } = this.props; - const cols = allowDeletes - ? tableColumns.concat(['__actions']) - : tableColumns; - return expandFieldset ? ['__expand'].concat(cols) : cols; - } - toggleExpand(id: any) { - this.onCellChange(id, '__expanded', false); this.setState(prevState => ({ expandedColumns: { ...prevState.expandedColumns, @@ -234,95 +227,62 @@ export default class CRUDCollection extends PureComponent< })); } - sortColumn(col: string, sort = SortOrder.Unsorted) { + handleTableChange( + _pagination: TablePaginationConfig, + _filters: Record, + sorter: SorterResult | SorterResult[], + ) { + const columnSorter = Array.isArray(sorter) ? sorter[0] : sorter; + let newSortColumn = ''; + let newSortOrder = 0; + + if (columnSorter?.columnKey && columnSorter?.order) { + newSortColumn = columnSorter.columnKey as string; + newSortOrder = columnSorter.order === 'ascend' ? 1 : 2; + } + const { sortColumns } = this.props; - // default sort logic sorting string, boolean and number - const compareSort = (m: Sort, n: Sort) => { - if (typeof m === 'string') { - return (m || ' ').localeCompare(n); - } - return m - n; - }; - return () => { - if (sortColumns?.includes(col)) { - // display in unsorted order if no sort specified - if (sort === SortOrder.Unsorted) { - const { collection } = createKeyedCollection(this.props.collection); - const collectionArray = createCollectionArray(collection); - this.setState({ - collectionArray, - sortColumn: '', - sort, - }); - return; + const col = newSortColumn; + + if (sortColumns?.includes(col) || newSortOrder === 0) { + let sortedArray = [...this.props.collection]; + + if (newSortOrder !== 0) { + const compareSort = (m: Sort, n: Sort) => { + if (typeof m === 'string' && typeof n === 'string') { + return (m || '').localeCompare(n || ''); + } + if (typeof m === 'number' && typeof n === 'number') { + return m - n; + } + if (typeof m === 'boolean' && typeof n === 'boolean') { + return m === n ? 0 : m ? 1 : -1; + } + const mStr = String(m ?? ''); + const nStr = String(n ?? ''); + return mStr.localeCompare(nStr); + }; + + sortedArray.sort((a: any, b: any) => compareSort(a[col], b[col])); + if (newSortOrder === 2) { + sortedArray.reverse(); } - - // newly ordered collection - const sorted = [...this.state.collectionArray].sort( - (a: Record, b: Record) => - compareSort(a[col], b[col]), + } else { + const { collectionArray } = createKeyedCollection( + this.props.collection, ); - const newCollection = - sort === SortOrder.Asc ? sorted : sorted.reverse(); - - this.setState(prevState => ({ - ...prevState, - collectionArray: newCollection, - sortColumn: col, - sort, - })); + sortedArray = collectionArray; } - }; - } - renderSortIcon(col: string) { - if (this.state.sortColumn === col && this.state.sort === SortOrder.Asc) { - return ; + this.setState({ + collectionArray: sortedArray, + sortColumn: newSortColumn, + sort: newSortOrder, + }); } - if (this.state.sortColumn === col && this.state.sort === SortOrder.Desc) { - return ; - } - return ; } - renderTH(col: string, sortColumns: Array) { - const tooltip = this.getTooltip(col); - return ( - - {this.getLabel(col)} - {tooltip && ( - <> - {' '} - - - )} - {sortColumns?.includes(col) && this.renderSortIcon(col)} - - ); - } - - renderHeaderRow() { - const cols = this.effectiveTableColumns(); - const { allowDeletes, expandFieldset, extraButtons, sortColumns } = - this.props; - return ( - - - {expandFieldset && } - {cols.map(col => this.renderTH(col, sortColumns))} - {extraButtons} - {allowDeletes && ( - - )} - - - ); - } - - renderExpandableSection(item: any) { + renderExpandableSection(item: any): ReactNode { const propsGenerator = () => ({ item, onChange: this.onFieldsetChange }); return recurseReactClone( this.props.expandFieldset, @@ -331,109 +291,116 @@ export default class CRUDCollection extends PureComponent< ); } - getCellProps(record: any, col: any) { - const cellPropsFn = this.props.itemCellProps?.[col]; - const val = record[col]; - return cellPropsFn ? cellPropsFn(val, this.getLabel(col), record) : {}; - } - - renderCell(record: any, col: any) { + renderCell(record: any, col: any): ReactNode { const renderer = this.props.itemRenderers?.[col]; const val = record[col]; const onChange = this.onCellChange.bind(this, record.id, col); return renderer ? renderer(val, onChange, this.getLabel(col), record) : val; } - renderItem(record: any) { - const { allowAddItem, allowDeletes, expandFieldset, tableColumns } = - this.props; - /* eslint-disable no-underscore-dangle */ - const isExpanded = - !!this.state.expandedColumns[record.id] || record.__expanded; - let tds = []; - if (expandFieldset) { - tds.push( - - - , - ); - } - tds = tds.concat( - tableColumns.map(col => ( - - {this.renderCell(record, col)} - - )), - ); - if (allowAddItem) { - tds.push(); - } + buildTableColumns() { + const { tableColumns, allowDeletes, sortColumns = [] } = this.props; + + const antdColumns: ColumnsType = tableColumns.map(col => { + const label = this.getLabel(col); + const tooltip = this.getTooltip(col); + const isSortable = sortColumns.includes(col); + const currentSortOrder: SortOrder | null | undefined = + this.state.sortColumn === col + ? this.state.sort === 1 + ? 'ascend' + : this.state.sort === 2 + ? 'descend' + : null + : null; + + return { + key: col, + dataIndex: col, + minWidth: 100, + title: ( + <> + {label} + {tooltip && ( + <> + {' '} + + + )} + + ), + render: (text: any, record: CollectionItem) => + this.renderCell(record, col), + onCell: (record: CollectionItem) => { + const cellPropsFn = this.props.itemCellProps?.[col]; + const val = record[col]; + return cellPropsFn ? cellPropsFn(val, label, record) : {}; + }, + sorter: isSortable, + sortOrder: currentSortOrder, + }; + }); + if (allowDeletes) { - tds.push( - - - , - ); - } - const trs = [ - - {tds} - , - ]; - if (isExpanded) { - trs.push( - - , + onCell: () => ({}), + sortOrder: null, + minWidth: 50, + render: (_, record: CollectionItem) => ( + -
{this.renderExpandableSection(record)}
- - , - ); + this.deleteItem(record.id)} + iconSize="l" + /> +
+ ), + }); } - return trs; - } - renderEmptyCell() { - return ( - - {this.props.emptyMessage} - - ); - } - - renderTableBody() { - const data = this.state.collectionArray; - const content = data.length - ? data.map(d => this.renderItem(d)) - : this.renderEmptyCell(); - return {content}; + return antdColumns as ColumnsType; } render() { + const { + stickyHeader, + emptyMessage = t('No items'), + expandFieldset, + } = this.props; + + const tableColumns = this.buildTableColumns(); + const expandedRowKeys = Object.keys(this.state.expandedColumns).filter( + id => this.state.expandedColumns[id], + ); + + const expandableConfig = expandFieldset + ? { + expandedRowRender: (record: CollectionItem) => + this.renderExpandableSection(record), + rowExpandable: () => true, + expandedRowKeys, + onExpand: (expanded: boolean, record: CollectionItem) => { + this.toggleExpand(record.id); + }, + } + : undefined; + return ( <> @@ -454,15 +421,26 @@ export default class CRUDCollection extends PureComponent< )} - - - {this.renderHeaderRow()} - {this.renderTableBody()} -
-
+ + data-test="crud-table" + columns={tableColumns} + data={this.state.collectionArray as CollectionItem[]} + rowKey={(record: CollectionItem) => String(record.id)} + sticky={stickyHeader} + pagination={false} + onChange={this.handleTableChange} + locale={{ emptyText: emptyMessage }} + css={ + stickyHeader && + css` + height: 350px; + overflow: auto; + ` + } + expandable={expandableConfig} + size={TableSize.Middle} + tableLayout="auto" + /> ); } diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx index 6b5e8d21b4c..9d9a352e62b 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.jsx @@ -433,9 +433,7 @@ function ColumnCollectionTable({ ), main_dttm_col: (value, _onItemChange, _label, record) => { const checked = datasource.main_dttm_col === record.column_name; - const disabled = !columns.find( - column => column.column_name === record.column_name, - ).is_dttm; + const disabled = !record?.is_dttm; return ( { const checked = datasource.main_dttm_col === record.column_name; - const disabled = !columns.find( - column => column.column_name === record.column_name, - ).is_dttm; + const disabled = !record?.is_dttm; return ( '), - __expanded: true, + expanded: true, })} /> diff --git a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx index 6b88e55f549..44dd3a4857c 100644 --- a/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx +++ b/superset-frontend/src/components/Datasource/DatasourceEditor.test.jsx @@ -94,7 +94,7 @@ describe('DatasourceEditor', () => { userEvent.click(columnsTab); const getToggles = screen.getAllByRole('button', { - name: /toggle expand/i, + name: /expand row/i, }); userEvent.click(getToggles[0]); const getTextboxes = screen.getAllByRole('textbox'); @@ -120,7 +120,7 @@ describe('DatasourceEditor', () => { userEvent.click(columnsTab); const getToggles = screen.getAllByRole('button', { - name: /toggle expand/i, + name: /expand row/i, }); userEvent.click(getToggles[0]); @@ -201,7 +201,7 @@ describe('DatasourceEditor RTL', () => { await asyncRender(props); const metricButton = screen.getByTestId('collection-tab-Metrics'); userEvent.click(metricButton); - const expandToggle = await screen.findAllByLabelText(/toggle expand/i); + const expandToggle = await screen.findAllByLabelText(/expand row/i); userEvent.click(expandToggle[0]); const certificationDetails = await screen.findByPlaceholderText( /certification details/i, @@ -227,7 +227,7 @@ describe('DatasourceEditor RTL', () => { await asyncRender(propsWithCurrency); const metricButton = screen.getByTestId('collection-tab-Metrics'); userEvent.click(metricButton); - const expandToggle = await screen.findAllByLabelText(/toggle expand/i); + const expandToggle = await screen.findAllByLabelText(/expand row/i); userEvent.click(expandToggle[0]); expect(await screen.findByText('Metric currency')).toBeVisible(); @@ -304,7 +304,7 @@ describe('DatasourceEditor RTL', () => { await asyncRender(props); const metricButton = screen.getByTestId('collection-tab-Metrics'); userEvent.click(metricButton); - const expandToggle = await screen.findAllByLabelText(/toggle expand/i); + const expandToggle = await screen.findAllByLabelText(/expand row/i); userEvent.click(expandToggle[1]); const certifiedBy = await screen.findByPlaceholderText(/certified by/i); userEvent.type(certifiedBy, 'I am typing a new name'); diff --git a/superset-frontend/src/components/Datasource/types.ts b/superset-frontend/src/components/Datasource/types.ts index b5acefe880d..62cd19921dd 100644 --- a/superset-frontend/src/components/Datasource/types.ts +++ b/superset-frontend/src/components/Datasource/types.ts @@ -70,6 +70,7 @@ export interface CRUDCollectionProps { ) => ReactNode)[]; onChange?: (arg0: any) => void; tableColumns: any[]; + tableLayout?: 'fixed' | 'auto'; sortColumns: string[]; stickyHeader?: boolean; } diff --git a/superset-frontend/src/components/Table/index.tsx b/superset-frontend/src/components/Table/index.tsx index ea6738f10dd..3da7374cee8 100644 --- a/superset-frontend/src/components/Table/index.tsx +++ b/superset-frontend/src/components/Table/index.tsx @@ -44,6 +44,8 @@ export enum ETableAction { } export type { ColumnsType }; +export type { TablePaginationConfig } from 'antd-v5/es/table'; +export type { SorterResult } from 'antd-v5/es/table/interface'; export type OnChangeFunction = AntTableProps['onChange'];