massucattoj commented on code in PR #40410:
URL: https://github.com/apache/superset/pull/40410#discussion_r3330546984
##########
superset-frontend/packages/superset-core/src/theme/GlobalStyles.tsx:
##########
@@ -115,6 +115,21 @@ export const GlobalStyles = () => {
display: flex;
margin-top: ${theme.marginXS}px;
}
+
+ .superset-explore-popover.ant-popover
+ .ant-popover-inner:has(.ant-popover-title) {
+ padding-top: 0;
+ }
+ .superset-explore-popover.ant-popover .ant-popover-title {
+ padding-top: ${theme.paddingXS}px;
+ margin-bottom: ${theme.paddingSM}px;
+ line-height: 1;
Review Comment:
@msyavuz @EnxDev replying here because pretty much you guys did the same
review
A few alternatives I looked at while working on this but didn't pan out:
- `styled(Popover)` doesn't work — the popover renders into `document.body`
via portal, so the styled className lands on the trigger, not on the actual
popover content.
- `ConfigProvider.components.Popover` doesn't either — `titleMarginBottom`
is an antd internal token (called out in the ticket's root cause), not exposed
in the theme API.
- The `styles` prop on antd v5.26 Popover only has semantic keys for `root`
and `body`, nothing for `title`, so I can't fully replace these rules via React
props.
The styles are scoped via `rootClassName="superset-explore-popover"` on
`ControlPopover`, so they only apply to explore control popovers — not actually
global in scope, just lives in
that file. Same pattern `GlobalStyles.tsx` already uses for color-picker,
column-config-popover, etc.
Open to ideas if you see a path I missed!
--
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]
