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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
