You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2022/11/16 00:56:49 UTC
[GitHub] [superset] eric-briscoe opened a new pull request, #22135: Adds virtualization option to antd based Table component
eric-briscoe opened a new pull request, #22135:
URL: https://github.com/apache/superset/pull/22135
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
This PR adds a `virtualization` prop to the recently added ant based Table component. Virtualization allows the tables to be used in cases where there are hundreds or thousands of columns, or large dataset with many thousands of rows with no pagination enabled.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
1. Checkout PR and open console at superset/superset-frontend
2. run `npm ci`
3. run `npm run storybook` and view docs and test virtualized table at: http://localhost:6006/?path=/story/design-system-components-table-overview--page under the "Virtualization for Performance" section, or interact directly with virtualized table instance at: http://localhost:6006/?path=/story/design-system-components-table-examples--virtualized-performance
4. Ensure the other table example stories work correctly
5. run `npm run dev-server` and go to http://localhost:9000/dataset/add/?testing to ensure the table instance used in the create dataset workflow still behaves as expected following instructions on this prior PR: https://github.com/apache/superset/pull/21948
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [X] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eric-briscoe commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026977162
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
+ pagination: TablePaginationConfig,
Review Comment:
@michael-s-molina this is a reflection of how antd provides access to these events. See the onChange docs at : https://ant.design/components/table/#API. I can provide an abstraction inside of table where externally on Table I can expose onPageChange, onFilterChange, onSortChange as separate events, but I think antd decided to pass all of that data in the single onChange handler because you can consolidate it all into one fetch call that includes the page info, filter info, and sorting info. If you capture these in separate events you would need to hold the state for the other events types so you can form a cohesive call to fetch the data combining the sort, filter, and page info.
All that said, when the table runs with the body set to virtual grid (following antd example) the onChange event actual errors out internally in antd on page change because it tries to invoke a scrollToTop call on a dom element that only exists when an override is not provided to antd table body. The onChange works correctly for filter and sort changes but I had to put in a workaround for the antd error. Luckily for pageChange there is a secondary place you can subscribe to an onChange inside of the paginationOptions. The down side is it only provides the page and pageSize values so my invocation of the onChange provided to our Table has empty objects for the sort and filter info at the moment... I think I know how I can resolve this.
After sharing all that, do you see the value in the single onChange having access to page, sort, and filter info, or still prefer granular event listener options?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1328014668
@eric-briscoe @michael-s-molina
I found a bug when I use `virtualize` and `usePagination={false}` at the same time, I still see pagination!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1320537408
Can we eliminate the scroll space of the table when there's no scroll?
<img width="1456" alt="Screen Shot 2022-11-18 at 4 21 43 PM" src="https://user-images.githubusercontent.com/70410625/202804856-cd5e1cb3-8b66-4198-b7cb-b1cf41f00785.png">
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025411756
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -292,32 +319,51 @@ export function Table(props: TableProps) {
* The exclusion from the effect dependencies is intentional and should not be modified
*/
// eslint-disable-next-line react-hooks/exhaustive-deps
- }, [wrapperRef, reorderable, resizable, interactiveTableUtils]);
+ }, [wrapperRef, reorderable, resizable, virtualize, interactiveTableUtils]);
const theme = useTheme();
+ const paginationSettings = usePagination
+ ? {
+ hideOnSinglePage: true,
+ pageSize,
+ pageSizeOptions,
+ onShowSizeChange: (page: number, size: number) => setPageSize(size),
+ }
+ : false;
return (
<ConfigProvider renderEmpty={renderEmpty}>
<div ref={wrapperRef}>
- <StyledTable
- {...rest}
- loading={{ spinning: loading ?? false, indicator: <Loading /> }}
- hasData={hideData ? false : data}
- rowSelection={selectionTypeValue ? rowSelection : undefined}
- columns={derivedColumns}
- dataSource={hideData ? [undefined] : data}
- size={size}
- sticky={sticky}
- pagination={{
- hideOnSinglePage: true,
- pageSize,
- pageSizeOptions,
- onShowSizeChange: (page: number, size: number) => setPageSize(size),
- }}
- showSorterTooltip={false}
- locale={mergedLocale}
- theme={theme}
- />
+ {!virtualize && (
+ <StyledTable
+ {...rest}
+ loading={{ spinning: loading ?? false, indicator: <Loading /> }}
+ hasData={hideData ? false : data}
+ rowSelection={selectionTypeValue ? rowSelection : undefined}
+ columns={derivedColumns}
+ dataSource={hideData ? [undefined] : data}
+ size={size}
+ sticky={sticky}
+ pagination={paginationSettings}
+ showSorterTooltip={false}
+ locale={mergedLocale}
+ theme={theme}
+ />
+ )}
+ {virtualize && (
+ <StyledVirtualTable
+ loading={{ spinning: loading ?? false, indicator: <Loading /> }}
+ hasData={hideData ? false : data}
+ columns={derivedColumns}
+ dataSource={hideData ? [undefined] : data}
+ size={size}
+ scroll={{ y: 300, x: '100vw' }}
+ theme={theme}
+ height={height}
+ pagination={paginationSettings}
+ locale={mergedLocale}
+ />
+ )}
Review Comment:
```suggestion
{!virtualize && (
<StyledTable
{...rest}
{...sharedProps}
rowSelection={selectionTypeValue ? rowSelection : undefined}
sticky={sticky}
showSorterTooltip={false}
theme={theme}
/>
)}
{virtualize && (
<StyledVirtualTable
{...sharedProps}
scroll={{ y: 300, x: '100vw' }}
theme={theme}
height={height}
/>
)}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eric-briscoe commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025806507
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,27 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Virtualization for Performance
+
+The Table virtualization can be used to enable viewing data with many columns and or rows without paging.
+Virtualization can be enabled via the `virtualize` prop. Pagination is shown in this example but can be truned off by setting `pagination={false}`
Review Comment:
nice catch!
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,27 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Virtualization for Performance
+
+The Table virtualization can be used to enable viewing data with many columns and or rows without paging.
Review Comment:
makes sense
##########
superset-frontend/src/components/Table/Table.stories.tsx:
##########
@@ -286,31 +321,37 @@ Basic.args = {
columns: basicColumns,
size: TableSize.SMALL,
onRow: handlers,
+ usePagination: false,
+};
+
+export const Pagination: ComponentStory<typeof Table> = args => (
+ <Table {...args} />
+);
+
+Pagination.args = {
+ data: basicData,
+ columns: basicColumns,
+ size: TableSize.SMALL,
pageSizeOptions: ['5', '10', '15', '20', '25'],
- defaultPageSize: 10,
+ defaultPageSize: 5,
};
-export const ManyColumns: ComponentStory<typeof Table> = args => (
- <ThemeProvider theme={supersetTheme}>
- <div style={{ height: '350px' }}>
- <Table {...args} />
- </div>
- </ThemeProvider>
+export const VirtualizedPerformance: ComponentStory<typeof Table> = args => (
+ <Table {...args} />
);
-ManyColumns.args = {
+VirtualizedPerformance.args = {
data: bigdata,
columns: bigColumns,
size: TableSize.SMALL,
resizable: true,
reorderable: true,
height: 350,
+ virtualize: true,
Review Comment:
👍
##########
superset-frontend/src/components/Table/VirtualTable.tsx:
##########
@@ -0,0 +1,154 @@
+/**
+ * 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 { Table } from 'antd';
+import type { TableProps } from 'antd/es/table';
+import classNames from 'classnames';
+import ResizeObserver from 'rc-resize-observer';
+import React, { useEffect, useRef, useState } from 'react';
+import { VariableSizeGrid as Grid } from 'react-window';
+import styled from '@emotion/styled';
+
+// If a column definition has no width, react-window will use this as the default column width
+const DEFAULT_COL_WIDTH = 150;
+
+const StyledCell = styled.div`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ padding-left: 8px;
+ padding-right: 4px;
Review Comment:
thanks, Addressing use of theme.gridUnit in next commit
##########
superset-frontend/package.json:
##########
@@ -158,6 +158,7 @@
"polished": "^3.7.2",
"prop-types": "^15.7.2",
"query-string": "^6.13.7",
+ "rc-resize-observer": "^1.2.0",
Review Comment:
Verified this works well and excited to NOT add a new dependency - thanks for the suggestion! will add changes in next commit
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -167,6 +177,20 @@ const StyledTable: StyledComponent<any> = styled(AntTable)<any>`
`}
`;
+const StyledVirtualTable: StyledComponent<any> = styled(VirtualTable)<any>`
+ .virtual-table .ant-table-container:before,
+ .virtual-table .ant-table-container:after {
+ display: none;
+ }
+ .virtual-table-cell {
+ box-sizing: border-box;
+ padding: 16px;
Review Comment:
I will update with use of theme.gridUnit, thx!
##########
superset-frontend/package.json:
##########
@@ -158,6 +158,7 @@
"polished": "^3.7.2",
"prop-types": "^15.7.2",
"query-string": "^6.13.7",
+ "rc-resize-observer": "^1.2.0",
Review Comment:
Great idea, happy to remove new dependency!
##########
superset-frontend/src/components/Table/Table.stories.tsx:
##########
@@ -35,6 +35,13 @@ export default {
title: 'Design System/Components/Table/Examples',
component: Table,
argTypes: { onClick: { action: 'clicked' } },
+ decorators: [
+ Story => (
+ <ThemeProvider theme={supersetTheme}>
+ <Story />
+ </ThemeProvider>
+ ),
+ ],
Review Comment:
great, nice catch!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eric-briscoe commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1028621111
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
+ pagination: TablePaginationConfig,
Review Comment:
@michael-s-molina the extra.action is populated for the page change events when in virtualization mode in last commit
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026883469
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
Review Comment:
Can you split this into different events? We may have different behaviors depending on which parameter changed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025422583
##########
superset-frontend/src/components/Table/Table.stories.tsx:
##########
@@ -35,6 +35,13 @@ export default {
title: 'Design System/Components/Table/Examples',
component: Table,
argTypes: { onClick: { action: 'clicked' } },
+ decorators: [
+ Story => (
+ <ThemeProvider theme={supersetTheme}>
+ <Story />
+ </ThemeProvider>
+ ),
+ ],
Review Comment:
The theme is already provided [here](https://github.com/apache/superset/blob/394fb2f2d0e05f27ced88e8ff4fc6994696cab68/superset-frontend/.storybook/preview.jsx#L50).
```suggestion
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026868421
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,93 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Server Pagination
+
+The table be configured for async data fetching to get partial data sets while showng pagination controls that let the user navigate through data.
+To override the default pagin, which uses data.length to determine the record count, populate the `recordCount` prop with the total number for records
+contained in the dataset on the server that is being paged through. When the User navigates through the paged data it will invoke the `onChange` callback
+function enableing data fetching to occur when the user changes page.
Review Comment:
```suggestion
The table can be configured for async data fetching to get partial data sets while showing pagination controls that let the user navigate through data.
To override the default paging, which uses `data.length` to determine the record count, populate the `recordCount` prop with the total number of records
contained in the dataset on the server being paged through. When the user navigates through the paged data it will invoke the `onChange` callback
function enabling data fetching to occur when the user changes the page.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1027301753
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
+ pagination: TablePaginationConfig,
Review Comment:
Thank you for the context @eric-briscoe. I noticed that the `extra` parameter has an `action` property with possible values of `pagination | sort | filter` which will enable us to track granular changes if needed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1316391232
Hello @eric-briscoe. Thanks for this PR!
I have some questions about VirtualTable:
- There's `react-virtual` (5.3 kB) which 3 times less than `rc-resize-observer` (15.6 kB) and provides hook for this func copmonent. Should we use `react-virtual` for VirtualTable?
- Why do we use `antd based Table` instead of `react-table`. `react-table` already supports virtualization - [virtualized-infinite-scrolling](https://github.com/TanStack/table/blob/main/examples/react/virtualized-infinite-scrolling/src/main.tsx) and [virtualized-rows](https://github.com/TanStack/table/blob/main/examples/react/virtualized-rows/src/main.tsx)
- Can we also use the VirtualTable in [FilterableTable](https://github.com/apache/superset/blob/master/superset-frontend/src/components/FilterableTable/index.tsx#L693) for result data in SQL Lab
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025395368
##########
superset-frontend/package.json:
##########
@@ -158,6 +158,7 @@
"polished": "^3.7.2",
"prop-types": "^15.7.2",
"query-string": "^6.13.7",
+ "rc-resize-observer": "^1.2.0",
Review Comment:
Can you use `react-resize-detector` which is already a dependency?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1322174460
When using `virtualize` the `render` function is not being called which prevents me from formatting the content.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1325219281
@eric-briscoe We have the concept of column formatting as you can see in the screenshot below:
<img width="868" alt="Screen Shot 2022-11-23 at 9 45 37 AM" src="https://user-images.githubusercontent.com/70410625/203575842-74bd0c22-1342-4662-a2b8-53cd2f7490a4.png">
I'm wondering about the best approach to implement this. It looks pretty similar to the [filters](https://ant.design/components/table#components-table-demo-head) concept provided by Ant Design's table. If we follow the same pattern, we can have `formattings`, `defaultFormatting`, `onFormatting` properties on `ColumnsType` to create the popup selector. We would also need to provide a `TimeCell` renderer that can be used when rendering dates/timestamps/numbers. WDYT?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eric-briscoe commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1320424423
> Hello @eric-briscoe. Thanks for this PR! I have some questions about VirtualTable:
>
> * There's `react-virtual` (5.3 kB) which 3 times less than `rc-resize-observer` (15.6 kB) and provides hook for this func component. Should we use `react-virtual` for this VirtualTable?
> * Why do we use `antd based Table` instead of `react-table`. `react-table` already supports virtualization - [virtualized-infinite-scrolling](https://github.com/TanStack/table/blob/main/examples/react/virtualized-infinite-scrolling/src/main.tsx) and [virtualized-rows](https://github.com/TanStack/table/blob/main/examples/react/virtualized-rows/src/main.tsx)
> * Can we also use the VirtualTable in [FilterableTable](https://github.com/apache/superset/blob/master/superset-frontend/src/components/FilterableTable/index.tsx#L693) for result data in SQL Lab
@EugeneTorap Great questions!
@michael-s-molina noted in a diff comment on this PR we already have 'react-resize-detector' as a dependency used in components such as MetadataBar so I refactored to use that library in place of `rc-resize-observer` and this PR introduces no new dependencies now
Bigger picture, the introduction of src/components/Table based on antd is continuing of work started with https://github.com/apache/superset/issues/20159 (SIP-82) about more consistent theming, use of Ant Design components and establishing a formal design system using antd as a base for new components.
VitrualTable is never intended to be used directly. It is a sub component to enable the src/components/Table to operate in a virtualized mode if needed and implementation is based from this antd example, then modified for our needs: https://ant.design/components/table/#components-table-demo-virtual-list
If you check out PR and run storybook, you will see the documentation for using the new Table component, and in this PR docs on using the `virtualize` prop have been added.
As far as using a different library such as react-table I was asked to use antd for consistency in a single Table implementation so that features like column definitions, sort functions, cell renderers, data formats, etc can be consistent, and the UI controls are consistent wether we are using the Table with or without virtualization.
Regarding use in SqlLab, the goal is to replace eventually replace all tables across superset-frontend with the new src/components/Table (but never VirtualTable directly). Table uses VirtualTable as a sub component when the Table prop `virtualize` is set to true. This allows of are lot of the common logic between virtual and non-virtual rendering to be handled at the Table component level and not duplicated in VirtualTable.
I was asked to introduce the Table capability incrementally so that it can be used for the new dataset creation and editing workflows, fix issues in the drill to detail modal, and the target other high use areas of the app to start swapping out. Some Table features will be need to be exposed / added as we do this and features refined as it gets used more. I was also asked to make Table an abstraction to antd so that it can leverage antd components but streamline usage for Superset use cases and not require developer to understand / use antd directly. That is bigger picture goal for design system which will likely be a follow on SIP to SIP-82, but still doing some research / prototyping to get this ready for community discussion.
I would really appreciate more eyes on the storybook examples and documentation so we can improve it to make usage as clear and efficient as possible for all Superset contributors. If you have any time to take a look and have feedback please send my way!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] EugeneTorap commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
EugeneTorap commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1320439015
@eric-briscoe Thank you for your answer! I like what you do!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026883469
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
Review Comment:
Can you split this into different events? We may have different behaviors depending on which property changed.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025411059
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -292,32 +319,51 @@ export function Table(props: TableProps) {
* The exclusion from the effect dependencies is intentional and should not be modified
*/
// eslint-disable-next-line react-hooks/exhaustive-deps
- }, [wrapperRef, reorderable, resizable, interactiveTableUtils]);
+ }, [wrapperRef, reorderable, resizable, virtualize, interactiveTableUtils]);
const theme = useTheme();
+ const paginationSettings = usePagination
+ ? {
+ hideOnSinglePage: true,
+ pageSize,
+ pageSizeOptions,
+ onShowSizeChange: (page: number, size: number) => setPageSize(size),
+ }
+ : false;
Review Comment:
```suggestion
const paginationSettings = usePagination
? {
hideOnSinglePage: true,
pageSize,
pageSizeOptions,
onShowSizeChange: (page: number, size: number) => setPageSize(size),
}
: false;
const sharedProps = {
loading: { spinning: loading ?? false, indicator: <Loading /> },
hasData: hideData ? false : data,
columns: derivedColumns,
dataSource: hideData ? [undefined] : data,
size,
pagination: paginationSettings,
locale: mergedLocale,
};
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025436804
##########
superset-frontend/src/components/Table/Table.stories.tsx:
##########
@@ -286,31 +321,37 @@ Basic.args = {
columns: basicColumns,
size: TableSize.SMALL,
onRow: handlers,
+ usePagination: false,
+};
+
+export const Pagination: ComponentStory<typeof Table> = args => (
+ <Table {...args} />
+);
+
+Pagination.args = {
+ data: basicData,
+ columns: basicColumns,
+ size: TableSize.SMALL,
pageSizeOptions: ['5', '10', '15', '20', '25'],
- defaultPageSize: 10,
+ defaultPageSize: 5,
};
-export const ManyColumns: ComponentStory<typeof Table> = args => (
- <ThemeProvider theme={supersetTheme}>
- <div style={{ height: '350px' }}>
- <Table {...args} />
- </div>
- </ThemeProvider>
+export const VirtualizedPerformance: ComponentStory<typeof Table> = args => (
+ <Table {...args} />
);
-ManyColumns.args = {
+VirtualizedPerformance.args = {
data: bigdata,
columns: bigColumns,
size: TableSize.SMALL,
resizable: true,
reorderable: true,
height: 350,
+ virtualize: true,
Review Comment:
I think this story is better represented if pagination is off by default.
```suggestion
virtualize: true,
usePagination: false,
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina merged pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina merged PR #22135:
URL: https://github.com/apache/superset/pull/22135
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1025418956
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,27 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Virtualization for Performance
+
+The Table virtualization can be used to enable viewing data with many columns and or rows without paging.
Review Comment:
I think we can disassociate virtualization and pagination. You can have a paginated table with column virtualization.
```suggestion
Table virtualization can enable viewing data with many columns and/or rows.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1023407724
##########
superset-frontend/src/components/Table/VirtualTable.tsx:
##########
@@ -0,0 +1,154 @@
+/**
+ * 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 { Table } from 'antd';
+import type { TableProps } from 'antd/es/table';
+import classNames from 'classnames';
+import ResizeObserver from 'rc-resize-observer';
+import React, { useEffect, useRef, useState } from 'react';
+import { VariableSizeGrid as Grid } from 'react-window';
+import styled from '@emotion/styled';
+
+// If a column definition has no width, react-window will use this as the default column width
+const DEFAULT_COL_WIDTH = 150;
+
+const StyledCell = styled.div`
+ white-space: nowrap;
+ overflow: hidden;
+ text-overflow: ellipsis;
+ padding-left: 8px;
+ padding-right: 4px;
Review Comment:
```suggestion
const DEFAULT_COL_WIDTH = ${supersetTheme.gridUnit * 37.5}px;
const StyledCell = styled.div`
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
padding-left: ${supersetTheme.gridUnit * 2}px;
padding-right: ${supersetTheme.gridUnit}px;
```
These should use the superset theme's gridunit for sizing.
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,27 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Virtualization for Performance
+
+The Table virtualization can be used to enable viewing data with many columns and or rows without paging.
+Virtualization can be enabled via the `virtualize` prop. Pagination is shown in this example but can be truned off by setting `pagination={false}`
Review Comment:
```suggestion
Virtualization can be enabled via the `virtualize` prop. Pagination is shown in this example but can be turned off by setting `pagination={false}`
```
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -167,6 +177,20 @@ const StyledTable: StyledComponent<any> = styled(AntTable)<any>`
`}
`;
+const StyledVirtualTable: StyledComponent<any> = styled(VirtualTable)<any>`
+ .virtual-table .ant-table-container:before,
+ .virtual-table .ant-table-container:after {
+ display: none;
+ }
+ .virtual-table-cell {
+ box-sizing: border-box;
+ padding: 16px;
Review Comment:
```suggestion
padding: ${supersetTheme.gridUnit * 4}px;
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1325075471
@eric-briscoe Can you add the behavior to scroll to the top when paginating?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026868421
##########
superset-frontend/src/components/Table/Table.overview.mdx:
##########
@@ -183,14 +183,93 @@ The table displays a set number of rows at a time, the user navigates the table
The default page size and page size options for the menu are configurable via the `pageSizeOptions` and `defaultPageSize` props.
NOTE: Pagination controls will only display when the data for the table has more records than the default page size.
-<Story id="design-system-components-table-examples--many-columns" />
+<Story id="design-system-components-table-examples--pagination" />
```
<Table pageSizeOptions={[5, 10, 15, 20, 25] defaultPageSize={10} />
```
---
+### Server Pagination
+
+The table be configured for async data fetching to get partial data sets while showng pagination controls that let the user navigate through data.
+To override the default pagin, which uses data.length to determine the record count, populate the `recordCount` prop with the total number for records
+contained in the dataset on the server that is being paged through. When the User navigates through the paged data it will invoke the `onChange` callback
+function enableing data fetching to occur when the user changes page.
Review Comment:
```suggestion
The table can be configured for async data fetching to get partial data sets while showing pagination controls that let the user navigate through data.
To override the default paging, which uses `data.length` to determine the record count, populate the `recordCount` prop with the total number of records contained in the dataset on the server being paged through. When the user navigates through the paged data it will invoke the `onChange` callback function enabling data fetching to occur when the user changes the page.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on a diff in pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on code in PR #22135:
URL: https://github.com/apache/superset/pull/22135#discussion_r1026886614
##########
superset-frontend/src/components/Table/index.tsx:
##########
@@ -134,6 +146,32 @@ export interface TableProps extends AntTableProps<TableProps> {
* when the number of rows exceeds the visible space.
*/
height?: number;
+ /**
+ * Sets the table to use react-window for scroll virtualization in cases where
+ * there are unknown amount of columns, or many, many rows
+ */
+ virtualize?: boolean;
+ /**
+ * Used to override page controls total record count when using server-side paging.
+ */
+ recordCount?: number;
+ /**
+ * Invoked when the tables sorting, paging, or filtering is changed.
+ */
+ onChange?: (
+ pagination: TablePaginationConfig,
Review Comment:
I didn't expect to receive a configuration object as the event value. I was expecting something like `page` and `pageSize`. Is there a reason for this type?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eric-briscoe commented on pull request #22135: feat: Adds virtualization option to antd based Table component
Posted by GitBox <gi...@apache.org>.
eric-briscoe commented on PR #22135:
URL: https://github.com/apache/superset/pull/22135#issuecomment-1322815117
> When using `virtualize` the `render` function is not being called which prevents me from formatting the content.
@michael-s-molina support for the render function on column definitions when in virtualized mode has bee added in latest commit
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org