rusackas commented on code in PR #45604:
URL: https://github.com/apache/superset/pull/45604#discussion_r4369347788
##########
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/GroupByFilterCard.test.tsx:
##########
@@ -532,3 +532,28 @@ test('shows no message while at least one allowed column
remains', async () => {
expect(await screen.findByText('Country')).toBeInTheDocument();
expect(screen.queryByText(allowlistGoneMessage)).not.toBeInTheDocument();
});
+
+test('horizontal layout keeps the 200-400px popup width constraints', async ()
=> {
+ // The popup portals to document.body, outside HorizontalFormItem's styled
+ // scope, so the width clamp must arrive as inline popup styles instead.
+ fetchMock.get('glob:*/api/v1/dataset/325', {
+ result: { table_name: 'Vehicle Sales', columns: [{ column_name: 'city' }]
},
+ });
+
+ render(
+ <GroupByFilterCard
+ customizationItem={customization([{ datasetId: 325 }])}
+ orientation="horizontal"
+ />,
+ { useRedux: true, initialState },
+ );
+
+ await userEvent.click(await screen.findByRole('combobox'));
+ await waitFor(() => {
+ expect(document.querySelector('.ant-select-dropdown')).not.toBeNull();
+ });
+ expect(document.querySelector('.ant-select-dropdown')).toHaveStyle({
+ minWidth: '200px',
Review Comment:
Added a test that renders the horizontal card inside an `overflow: auto`
wrapper and asserts the opened dropdown is outside it. Not run locally, so CI
is the first real run.
##########
superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/controls/ColorSchemeControl/index.tsx:
##########
@@ -304,28 +305,40 @@ const ColorSchemeControl = ({
/>
}
/>
- <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}
- disabled={hasDashboardScheme || hasSharedLabelsColor}
- onChange={handleOnChange}
- 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}
+ disabled={hasDashboardScheme || hasSharedLabelsColor}
+ onChange={handleOnChange}
+ placeholder={t('Select scheme')}
+ value={currentScheme}
+ showSearch
+ getPopupContainer={() => document.body}
Review Comment:
Added a grouped-palette test here that asserts the popup is outside the
control container and that the grouped-option rule is attached to the popup
root class. Not run locally, so CI is the first real run.
--
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]
