rusackas commented on code in PR #37777:
URL: https://github.com/apache/superset/pull/37777#discussion_r3488481012
##########
superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx:
##########
@@ -188,14 +188,22 @@ export const DataTablesPane = ({
queryForce,
ownState,
isRequest: isRequest.results,
- setForceQuery,
- isVisible: ResultTypes.Results === activeTabKey,
+ actions,
Review Comment:
There's no `actions` in this component at all, so I think this one's a false
positive from the bot. Nothing to remove here :)
##########
superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx:
##########
@@ -188,14 +188,21 @@ export const DataTablesPane = ({
queryForce,
ownState,
isRequest: isRequest.results,
- setForceQuery,
- isVisible: ResultTypes.Results === activeTabKey,
canDownload,
- }).map((pane, idx) => ({
- key: idx === 0 ? ResultTypes.Results : `${ResultTypes.Results} ${idx + 1}`,
- label: idx === 0 ? t('Results') : t('Results %s', idx + 1),
- children: pane,
- }));
+ }).map((pane, idx) => {
+ const tabKey =
+ idx === 0 ? ResultTypes.Results : `${ResultTypes.Results} ${idx + 1}`;
+
+ return {
+ key: tabKey,
+ label: idx === 0 ? t('Results') : t('Results %s', idx + 1),
+ children:
+ activeTabKey === tabKey ? (
+ pane
+ ) : null,
+ };
+ });
+
Review Comment:
Yeah, the filter text resets when you switch result tabs, but that seems
like a fine tradeoff to get the inactive tables to actually unmount. Won't lose
sleep over that one.
##########
superset-frontend/src/explore/components/DataTablesPane/DataTablesPane.tsx:
##########
@@ -188,14 +188,17 @@ export const DataTablesPane = ({
queryForce,
ownState,
isRequest: isRequest.results,
- setForceQuery,
- isVisible: ResultTypes.Results === activeTabKey,
canDownload,
Review Comment:
Good catch, this one was legit. The hook leans on `setForceQuery?.(false)`
to reset force mode after a query resolves, so I wired it back into the
`useResultsPane` call.
##########
superset-frontend/src/explore/components/DataTablesPane/components/useResultsPane.tsx:
##########
@@ -181,7 +181,7 @@ export const useResultsPane = ({
coltypes={result.coltypes}
rowcount={result.rowcount}
datasourceId={queryFormData.datasource}
- isVisible={isVisible}
+ isVisible={isVisible ?? true}
Review Comment:
`isVisible` is intentionally optional now... the parent unmounts inactive
panes, so a mounted pane is always the visible one, and the `?? true` fallback
covers it.
--
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]
