alexandrusoare commented on code in PR #38563:
URL: https://github.com/apache/superset/pull/38563#discussion_r2939212510
##########
superset-frontend/packages/superset-ui-core/src/components/TableView/TableView.tsx:
##########
@@ -117,43 +117,45 @@ const RawTableView = ({
...props
}: TableViewProps) => {
const tableRef = useRef<HTMLTableElement>(null);
+ const effectivePageSize = initialPageSize ?? DEFAULT_PAGE_SIZE;
+ const [pageIndex, setPageIndex] = useState(initialPageIndex ?? 0);
const initialState = useMemo(
() => ({
- pageSize: initialPageSize ?? DEFAULT_PAGE_SIZE,
- pageIndex: initialPageIndex ?? 0,
+ pageSize: effectivePageSize,
+ pageIndex: 0,
sortBy: initialSortBy,
}),
- [initialPageSize, initialPageIndex, initialSortBy],
+ [effectivePageSize, initialSortBy],
);
const {
getTableProps,
getTableBodyProps,
headerGroups,
- page,
rows,
prepareRow,
- gotoPage,
setSortBy,
- state: { pageIndex, sortBy },
+ state: { sortBy },
} = useTable(
{
columns,
data,
initialState,
- manualPagination: serverPagination,
+ manualPagination: true,
manualSortBy: serverPagination,
Review Comment:
Why did we decide to set TableView to `true` by default?
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.tsx:
##########
@@ -71,7 +71,7 @@ const FilterScope: FC<FilterScopeProps> = ({
);
const updateScopes = useCallback(
- updatedFormValues => {
+ (updatedFormValues: Record<string, any>) => {
Review Comment:
I see that for some we use `string, any` and for some we sue `string,
unknown`. What do you think of standardizing this, and use just one of them?
##########
superset-frontend/src/SqlLab/components/QueryTable/index.tsx:
##########
@@ -52,10 +52,10 @@ interface QueryTableQuery extends Omit<
QueryResponse,
'state' | 'sql' | 'progress' | 'results' | 'duration' | 'started'
> {
- state?: Record<string, any>;
- sql?: Record<string, any>;
- progress?: Record<string, any>;
- results?: Record<string, any>;
+ state?: ReactNode;
+ sql?: ReactNode;
+ progress?: ReactNode;
+ results?: ReactNode;
duration?: ReactNode;
started?: ReactNode;
Review Comment:
This seems relevant as well
##########
superset-frontend/src/features/roles/RoleListEditModal.test.tsx:
##########
@@ -145,6 +145,17 @@ describe('RoleListEditModal', () => {
render(<RoleListEditModal {...mockProps} />);
+ // Wait for user hydration to complete before submitting
+ await waitFor(() => {
+ expect(
+ (SupersetClient.get as jest.Mock).mock.calls.some(([call]) =>
+ call.endpoint.includes('/api/v1/security/users/'),
+ ),
+ ).toBe(true);
+ });
+ // Allow the setFieldsValue effect to fire after loading state updates
+ await waitFor(() => Promise.resolve());
Review Comment:
What do you think of this?
##########
superset-frontend/plugins/plugin-chart-ag-grid-table/src/AgGridTable/index.tsx:
##########
@@ -298,10 +298,12 @@ const AgGridDataTable:
FunctionComponent<AgGridTableProps> = memo(
};
const handleColumnHeaderClick = useCallback(
- params => {
+ (params: { column?: { colId?: string; sort?: string } }) => {
const colId = params?.column?.colId;
const sortDir = params?.column?.sort;
- handleColSort(colId, sortDir);
+ if (colId && sortDir) {
+ handleColSort(colId, sortDir);
+ }
Review Comment:
This seems relevant
--
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]
