EnxDev commented on PR #43454:
URL: https://github.com/apache/superset/pull/43454#issuecomment-5455217305
## EnxDev's Review Agent โ apache/superset#43454 ยท HEAD 3f61dd8
**comment** โ one-liner fix is correct and verified against `Tabs.tsx`, but
one live reviewer comment is unaddressed.
The PR went through a large detour (stale-tab reconciliation added, then
fully reverted across 4 commits, with the reconciliation split out to #43648)
and is now back to just the `fullHeight` fix. Confirmed against `Tabs.tsx`
(packages/superset-ui-core/src/components/Tabs/Tabs.tsx:49-66): `fullHeight`
sets `height: 100%` on `.ant-tabs`, `.ant-tabs-body-holder`, `.ant-tabs-body`,
and `.ant-tabs-content` โ exactly the chain the PR description says was broken.
The fix is correct and matches the stated root cause.
### ๐ก Should-fix
-
**`superset-frontend/src/explore/components/DataTablesPane/components/ResultsPaneOnDashboard.tsx:31-37`**
โ `Wrapper`'s `.ant-tabs { height: 100% }` and `.ant-tabs-body { height: 100%
}` rules are dead weight: `fullHeight` on `<Tabs>` already emits both
(`Tabs.tsx:52,63`). These were correctly dropped once, then reintroduced by the
revert of the stale-tab commits, and msyavuz flagged it again on this HEAD with
no reply yet. Drop the two rules.
### ๐ต Nits
-
`superset-frontend/src/explore/components/DataTablesPane/test/ResultsPaneOnDashboard.test.tsx:31-34`
โ call-args spy on `Tabs` is a reasonable workaround for the missing Jest
emotion `importSource`, but it only asserts the prop is *passed*, not that it
renders correctly. Fine given the documented Jest limitation; no action needed.
No functional or security findings โ small, well-scoped fix, test covers the
regression (asserts `fullHeight: true` is passed).
<!-- enxdev-review-agent:3f61dd8 -->
_Reviewed by EnxDev's Review Agent โ @EnxDev ยท HEAD 3f61dd8._
--
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]
