7487 commented on code in PR #39657:
URL: https://github.com/apache/superset/pull/39657#discussion_r3903165201
##########
superset-frontend/src/dashboard/components/ColorSchemeSelect.tsx:
##########
@@ -172,27 +173,39 @@ const ColorSchemeSelect = ({
return (
<>
- <Select
- css={css`
- width: 100%;
- & .ant-select-item.ant-select-item-group {
- padding-left: ${theme.sizeUnit}px;
- font-size: ${theme.fontSize}px;
- }
- & .ant-select-item-option-grouped {
- padding-left: ${theme.sizeUnit * 3}px;
- }
- `}
- aria-label={t('Select color scheme')}
- allowClear={clearable}
- onChange={onChange}
- placeholder={t('Select scheme')}
- value={currentScheme}
- showSearch
- getPopupContainer={triggerNode => triggerNode.parentNode}
- options={options}
- optionFilterProps={['label', 'value', 'searchText']}
- />
+ <ClassNames>
+ {({ css: popupCss }) => (
+ <Select
+ css={css`
+ width: 100%;
+ `}
+ // the popup portals to document.body, out of the css prop's
+ // scope, so the grouped-option styles ride along on its root
+ classNames={{
+ popup: {
+ root: popupCss`
+ .ant-select-item.ant-select-item-group {
+ padding-left: ${theme.sizeUnit}px;
+ font-size: ${theme.fontSize}px;
+ }
+ .ant-select-item-option-grouped {
+ padding-left: ${theme.sizeUnit * 3}px;
+ }
+ `,
+ },
+ }}
+ aria-label={t('Select color scheme')}
+ allowClear={clearable}
+ onChange={onChange}
+ placeholder={t('Select scheme')}
+ value={currentScheme}
+ showSearch
+ getPopupContainer={() => document.body}
+ options={options}
+ optionFilterProps={['label', 'value', 'searchText']}
+ />
+ )}
+ </ClassNames>
Review Comment:
The three-way duplication predates this PR — the identical `&
.ant-select-item...` block exists verbatim in all three files on master, and
the copies haven't drifted from each other. (The commit message describes the
styles no longer matching the popup after it portals to `document.body`, not
the copies diverging.) This PR just ports each existing copy to
`classNames.popup.root` mechanically, keeping the diff reviewable.
Extracting one helper reachable by all three is blocked by the package
boundary: the word-cloud control is a deliberate whole-file fork of the explore
control because plugin packages can't import from `src/`, so the helper would
have to become a new public `@superset-ui/core` export — an API addition that
feels out of scope for this bugfix. Sharing between only the two `src/` copies
would still leave two sources of truth. Happy to do the extraction as a
follow-up if maintainers want 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]
