ktmud commented on a change in pull request #13500:
URL: https://github.com/apache/superset/pull/13500#discussion_r595597689
##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -36,6 +36,7 @@ interface ListViewResourceState<D extends object = any> {
lastFetchDataConfig: FetchDataConfig | null;
bulkSelectEnabled: boolean;
lastFetched?: string;
+ defualtLoading?: boolean;
Review comment:
```suggestion
defaultLoading?: boolean;
```
Typo, plus this is probably not needed since the state itself doesn't really
use `defaultLoading`.
##########
File path: superset-frontend/src/views/CRUD/welcome/ActivityTable.tsx
##########
@@ -25,7 +25,7 @@ import ListViewCard from 'src/components/ListViewCard';
import SubMenu from 'src/components/Menu/SubMenu';
import { Chart } from 'src/types/Chart';
import { Dashboard, SavedQueryObject } from 'src/views/CRUD/types';
-import { mq, CardStyles } from 'src/views/CRUD/utils';
+import { mq, CardStyles, getEditedObjs } from 'src/views/CRUD/utils';
Review comment:
Nit: maybe it is just me, but I think `getEditedObjects` or
`getUserEditedEntities` would be more readable.
##########
File path: superset-frontend/src/views/CRUD/hooks.ts
##########
@@ -45,11 +46,12 @@ export function useListViewResource<D extends object = any>(
infoEnable = true,
defaultCollectionValue: D[] = [],
baseFilters?: FilterValue[], // must be memoized
+ defaultLoading = true,
Review comment:
Nit: can we call it `initialLoadingState`? This function also has too
many positional arguments. In general, I'd recommend refactoring any function
with >=3 arguments to use `options` object.
##########
File path: superset-frontend/src/views/CRUD/utils.tsx
##########
@@ -29,6 +29,12 @@ import { getClientErrorObject } from
'src/utils/getClientErrorObject';
import { FetchDataConfig } from 'src/components/ListView';
import { Dashboard } from './types';
+interface Filters {
+ col: string;
+ opr: string;
+ value: string;
+}
Review comment:
Nit: let's make this a shared type with a more specific name (e.g.
`CRUDFilterParam`) and put it into `views/CRUD/types.tsx`, as it is obviously
used in other components, too.
##########
File path: superset-frontend/src/views/CRUD/welcome/Welcome.tsx
##########
@@ -27,6 +27,7 @@ import {
createErrorHandler,
getRecentAcitivtyObjs,
mq,
+ getMineObjs,
Review comment:
How about `getUserOwnedObjects`?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
