Select: Portal menu by default (#48176)

* Remove menuShouldPortal from all <Select /> components

* fix unit tests

* leave menuShouldPortal as an escape hatch

* Fix import order
This commit is contained in:
Ashley Harrison
2022-05-04 15:12:59 +01:00
committed by GitHub
parent 2738d1c557
commit 06d3c27bc1
179 changed files with 67 additions and 372 deletions

View File

@@ -165,7 +165,6 @@ export class DataSourcePicker extends PureComponent<DataSourcePickerProps, DataS
<Select
aria-label={selectors.components.DataSourcePicker.inputV2}
inputId={inputId || 'data-source-picker'}
menuShouldPortal
className="ds-picker select-container"
isMulti={false}
isClearable={isClearable}

View File

@@ -212,7 +212,6 @@ export class Cascader extends React.PureComponent<CascaderProps, CascaderState>
<div>
{isSearching ? (
<Select
menuShouldPortal
allowCustomValue={allowCustomValue}
placeholder={placeholder}
autoFocus={!focusCascade}

View File

@@ -58,7 +58,6 @@ export function AlertingSettings<T extends AlertingConfig>({
>
<Select
width={29}
menuShouldPortal
options={alertmanagerOptions}
onChange={(value) =>
onOptionsChange({ ...options, jsonData: { ...options.jsonData, alertmanagerUid: value?.value } })

View File

@@ -109,7 +109,6 @@ export const DataSourceHttpSettings: React.FC<HttpSettingsProps> = (props) => {
const accessSelect = (
<Select
aria-label="Access"
menuShouldPortal
className="width-20 gf-form-input"
options={ACCESS_OPTIONS}
value={ACCESS_OPTIONS.filter((o) => o.value === dataSourceConfig.access)[0] || DEFAULT_ACCESS_OPTION}

View File

@@ -109,6 +109,7 @@ export const TimePickerFooter: FC<Props> = (props) => {
<Field className={style.fiscalYearField} label={'Fiscal year start month'}>
<Select
value={fiscalYearStartMonth}
menuShouldPortal={false}
options={monthOptions}
onChange={(value) => {
if (onChangeFiscalYearStartMonth) {

View File

@@ -60,6 +60,7 @@ export const TimeZonePicker: React.FC<Props> = (props) => {
value={selected}
placeholder="Type to search (country, city, abbreviation)"
autoFocus={autoFocus}
menuShouldPortal={false}
openMenuOnFocus={true}
width={width}
filterOption={filterBySearchIndex}

View File

@@ -47,7 +47,6 @@ export const WeekStartPicker: React.FC<Props> = (props) => {
onChange={onChangeWeekStart}
onBlur={onBlur}
disabled={disabled}
menuShouldPortal={true}
/>
);
};

View File

@@ -30,7 +30,7 @@ describe('Field', () => {
it('renders with the inputId of its children', () => {
render(
<Field label="My other label">
<Select menuShouldPortal inputId="my-select-input" onChange={() => {}} />
<Select inputId="my-select-input" onChange={() => {}} />
</Field>
);

View File

@@ -114,7 +114,7 @@ const renderForm = (defaultValues?: FormDTO) => (
rules={{
required: true,
}}
render={({ field }) => <Select menuShouldPortal {...field} options={selectOptions} />}
render={({ field }) => <Select {...field} options={selectOptions} />}
/>
</Field>

View File

@@ -45,7 +45,6 @@ export const withSelect = () => {
return (
<InlineField label="Select option">
<Select
menuShouldPortal
width={16}
onChange={action('item selected')}
options={[

View File

@@ -30,7 +30,7 @@ describe('InlineField', () => {
it('renders with the inputId of its children', () => {
render(
<InlineField label="My other label">
<Select menuShouldPortal inputId="my-select-input" onChange={() => {}} />
<Select inputId="my-select-input" onChange={() => {}} />
</InlineField>
);

View File

@@ -72,7 +72,6 @@ export const Basic: Story = (args) => {
{(value, updateValue) => {
return (
<Select
menuShouldPortal
{...args}
onChange={(value: SelectableValue<string>) => {
action('onChanged fired')(value);

View File

@@ -23,7 +23,7 @@ export const FieldNameMatcherEditor = memo<MatcherUIProps<string>>((props) => {
);
const selectedOption = selectOptions.find((v) => v.value === options);
return <Select menuShouldPortal value={selectedOption} options={selectOptions} onChange={onChange} inputId={id} />;
return <Select value={selectedOption} options={selectOptions} onChange={onChange} inputId={id} />;
});
FieldNameMatcherEditor.displayName = 'FieldNameMatcherEditor';

View File

@@ -31,7 +31,6 @@ export const FieldNamePicker: React.FC<StandardEditorProps<string, FieldNamePick
return (
<>
<Select
menuShouldPortal
value={selectedOption}
placeholder={settings.placeholderText ?? 'Select field'}
options={selectOptions}

View File

@@ -39,7 +39,7 @@ export const FieldNamesMatcherEditor = memo<MatcherUIProps<ByNamesMatcherOptions
return <Input value={displayNames} readOnly={true} disabled={true} prefix={prefix} />;
}
return <MultiSelect menuShouldPortal value={options.names} options={selectOptions} onChange={onChange} />;
return <MultiSelect value={options.names} options={selectOptions} onChange={onChange} />;
});
FieldNamesMatcherEditor.displayName = 'FieldNameMatcherEditor';

View File

@@ -19,7 +19,7 @@ export const FieldTypeMatcherEditor = memo<MatcherUIProps<string>>((props) => {
);
const selectedOption = selectOptions.find((v) => v.value === options);
return <Select inputId={id} value={selectedOption} options={selectOptions} onChange={onChange} menuShouldPortal />;
return <Select inputId={id} value={selectedOption} options={selectOptions} onChange={onChange} />;
});
FieldTypeMatcherEditor.displayName = 'FieldTypeMatcherEditor';

View File

@@ -26,7 +26,7 @@ export const FieldsByFrameRefIdMatcher = memo<MatcherUIProps<string>>((props) =>
);
const selectedOption = selectOptions.find((v) => v.value === options);
return <Select menuShouldPortal value={selectedOption} options={selectOptions} onChange={onChange} />;
return <Select value={selectedOption} options={selectOptions} onChange={onChange} />;
});
FieldsByFrameRefIdMatcher.displayName = 'FieldsByFrameRefIdMatcher';

View File

@@ -79,7 +79,6 @@ export const FieldColorEditor: React.FC<FieldConfigEditorProps<FieldColor | unde
return (
<div className={styles.group}>
<Select
menuShouldPortal
minMenuHeight={200}
options={options}
value={mode}
@@ -102,14 +101,7 @@ export const FieldColorEditor: React.FC<FieldConfigEditorProps<FieldColor | unde
return (
<>
<div style={{ marginBottom: theme.spacing(2) }}>
<Select
menuShouldPortal
minMenuHeight={200}
options={options}
value={mode}
onChange={onModeChange}
inputId={id}
/>
<Select minMenuHeight={200} options={options} value={mode} onChange={onModeChange} inputId={id} />
</div>
<Field label="Color series by">
<RadioButtonGroup value={value?.seriesBy ?? 'last'} options={seriesModes} onChange={onSeriesModeChange} />
@@ -118,9 +110,7 @@ export const FieldColorEditor: React.FC<FieldConfigEditorProps<FieldColor | unde
);
}
return (
<Select menuShouldPortal minMenuHeight={200} options={options} value={mode} onChange={onModeChange} inputId={id} />
);
return <Select minMenuHeight={200} options={options} value={mode} onChange={onModeChange} inputId={id} />;
};
interface ModeProps {

View File

@@ -61,7 +61,6 @@ export class MultiSelectValueEditor<T> extends React.PureComponent<Props<T>, Sta
const { settings } = item;
return (
<MultiSelect<T>
menuShouldPortal
isLoading={isLoading}
value={value}
defaultValue={value}

View File

@@ -64,7 +64,6 @@ export class SelectValueEditor<T> extends React.PureComponent<Props<T>, State<T>
}
return (
<Select<T>
menuShouldPortal
isLoading={isLoading}
value={current}
defaultValue={value}

View File

@@ -60,7 +60,6 @@ export function SegmentSelect<T>({
return (
<div {...rest} ref={ref}>
<Component
menuShouldPortal
width={width}
noOptionsMessage={noOptionsMessage}
placeholder={placeholder}

View File

@@ -94,7 +94,6 @@ export const Basic: Story<StoryProps> = (args) => {
return (
<>
<Select
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -114,7 +113,6 @@ export const BasicSelectPlainValue: Story<StoryProps> = (args) => {
return (
<>
<Select
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -148,7 +146,6 @@ export const SelectWithOptionDescriptions: Story = (args) => {
return (
<>
<Select
menuShouldPortal
options={options}
value={value}
onChange={(v) => {
@@ -171,7 +168,6 @@ export const MultiPlainValue: Story = (args) => {
return (
<>
<MultiSelect
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -190,7 +186,6 @@ export const MultiSelectWithOptionGroups: Story = (args) => {
return (
<>
<MultiSelect
menuShouldPortal
options={[
{ label: '1', value: '1' },
{ label: '2', value: '2', options: [{ label: '5', value: '5' }] },
@@ -213,7 +208,6 @@ export const MultiSelectBasic: Story = (args) => {
return (
<>
<MultiSelect
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -237,7 +231,6 @@ export const MultiSelectAsync: Story = (args) => {
return (
<AsyncMultiSelect
menuShouldPortal
loadOptions={loadAsyncOptions}
defaultOptions
value={value}
@@ -259,7 +252,6 @@ export const BasicSelectAsync: Story = (args) => {
return (
<AsyncSelect
menuShouldPortal
loadOptions={loadAsyncOptions}
defaultOptions
value={value}
@@ -280,7 +272,6 @@ export const AutoMenuPlacement: Story = (args) => {
<>
<div style={{ width: '100%', height: '95vh', display: 'flex', alignItems: 'flex-end' }}>
<Select
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -305,7 +296,6 @@ export const WidthAuto: Story = (args) => {
<>
<div style={{ width: '100%' }}>
<Select
menuShouldPortal
options={generateOptions()}
value={value}
onChange={(v) => {
@@ -328,7 +318,6 @@ export const CustomValueCreation: Story = (args) => {
return (
<>
<Select
menuShouldPortal
options={[...options, ...customOptions]}
value={value}
onChange={(v) => {

View File

@@ -21,11 +21,11 @@ describe('SelectBase', () => {
];
it('renders without error', () => {
render(<SelectBase menuShouldPortal onChange={onChangeHandler} />);
render(<SelectBase onChange={onChangeHandler} />);
});
it('renders empty options information', async () => {
render(<SelectBase menuShouldPortal onChange={onChangeHandler} />);
render(<SelectBase onChange={onChangeHandler} />);
await userEvent.click(screen.getByText(/choose/i));
expect(screen.queryByText(/no options found/i)).toBeVisible();
});
@@ -34,7 +34,7 @@ describe('SelectBase', () => {
render(
<>
<label htmlFor="my-select">My select</label>
<SelectBase menuShouldPortal onChange={onChangeHandler} options={options} inputId="my-select" />
<SelectBase onChange={onChangeHandler} options={options} inputId="my-select" />
</>
);
@@ -49,7 +49,7 @@ describe('SelectBase', () => {
return (
<>
<button onClick={() => setValue(null)}>clear value</button>
<SelectBase menuShouldPortal value={value} onChange={setValue} options={[option]} />
<SelectBase value={value} onChange={setValue} options={[option]} />
</>
);
};
@@ -63,7 +63,7 @@ describe('SelectBase', () => {
describe('when openMenuOnFocus prop', () => {
describe('is provided', () => {
it('opens on focus', () => {
render(<SelectBase menuShouldPortal onChange={onChangeHandler} openMenuOnFocus />);
render(<SelectBase onChange={onChangeHandler} openMenuOnFocus />);
fireEvent.focus(screen.getByRole('combobox'));
expect(screen.queryByText(/no options found/i)).toBeVisible();
});
@@ -75,7 +75,7 @@ describe('SelectBase', () => {
${'ArrowUp'}
${' '}
`('opens on arrow down/up or space', ({ key }) => {
render(<SelectBase menuShouldPortal onChange={onChangeHandler} />);
render(<SelectBase onChange={onChangeHandler} />);
fireEvent.focus(screen.getByRole('combobox'));
fireEvent.keyDown(screen.getByRole('combobox'), { key });
expect(screen.queryByText(/no options found/i)).toBeVisible();
@@ -114,7 +114,6 @@ describe('SelectBase', () => {
it('should only display maxVisibleValues options, and additional number of values should be displayed as indicator', () => {
render(
<SelectBase
menuShouldPortal
onChange={onChangeHandler}
isMulti={true}
maxVisibleValues={3}
@@ -131,7 +130,6 @@ describe('SelectBase', () => {
it('should show all selected options when menu is open', () => {
render(
<SelectBase
menuShouldPortal
onChange={onChangeHandler}
isMulti={true}
maxVisibleValues={3}
@@ -151,7 +149,6 @@ describe('SelectBase', () => {
it('should not show all selected options when menu is open', () => {
render(
<SelectBase
menuShouldPortal
onChange={onChangeHandler}
isMulti={true}
maxVisibleValues={3}
@@ -172,7 +169,6 @@ describe('SelectBase', () => {
it('should always show all selected options', () => {
render(
<SelectBase
menuShouldPortal
onChange={onChangeHandler}
isMulti={true}
options={excessiveOptions}
@@ -189,7 +185,7 @@ describe('SelectBase', () => {
describe('options', () => {
it('renders menu with provided options', async () => {
render(<SelectBase menuShouldPortal options={options} onChange={onChangeHandler} />);
render(<SelectBase options={options} onChange={onChangeHandler} />);
await userEvent.click(screen.getByText(/choose/i));
const menuOptions = screen.getAllByLabelText('Select option');
expect(menuOptions).toHaveLength(2);
@@ -198,7 +194,7 @@ describe('SelectBase', () => {
it('call onChange handler when option is selected', async () => {
const spy = jest.fn();
render(<SelectBase menuShouldPortal onChange={spy} options={options} aria-label="My select" />);
render(<SelectBase onChange={spy} options={options} aria-label="My select" />);
const selectEl = screen.getByLabelText('My select');
expect(selectEl).toBeInTheDocument();

View File

@@ -119,8 +119,7 @@ export function SelectBase<T>({
maxVisibleValues,
menuPlacement = 'auto',
menuPosition,
// TODO change this to default to true for Grafana 9
menuShouldPortal = false,
menuShouldPortal = true,
noOptionsMessage = 'No options found',
onBlur,
onChange,

View File

@@ -48,8 +48,7 @@ export interface SelectCommonProps<T> {
menuPlacement?: 'auto' | 'bottom' | 'top';
menuPosition?: 'fixed' | 'absolute';
/**
* Setting to true will portal the menu to `document.body`.
* This property will soon default to true and portalling will be the default behavior.
* Setting to false will prevent the menu from portalling to the body.
*/
menuShouldPortal?: boolean;
/** The message to display when no options could be found */

View File

@@ -68,7 +68,6 @@ export class StatsPicker extends PureComponent<Props> {
const select = fieldReducers.selectOptions(stats);
return (
<Select
menuShouldPortal
value={select.current}
className={className}
isClearable={!defaultStat}

View File

@@ -136,12 +136,7 @@ export const ThemeDemo = () => {
<Input placeholder="Placeholder" value="Disabled value" />
</Field>
<Field label="Select">
<Select
menuShouldPortal
options={selectOptions}
value={selectValue}
onChange={(v) => setSelectValue(v?.value!)}
/>
<Select options={selectOptions} value={selectValue} onChange={(v) => setSelectValue(v?.value!)} />
</Field>
<Field label="Radio label">
<RadioButtonGroup options={radioOptions} value={radioValue} onChange={setRadioValue} />

View File

@@ -62,7 +62,6 @@ export function ValuePicker<T>({
{isPicking && (
<span style={{ minWidth: theme.spacing(minWidth), flexGrow: isFullWidth ? 1 : undefined }}>
<Select
menuShouldPortal
placeholder={label}
options={options}
aria-label={selectors.components.ValuePicker.select(label)}

View File

@@ -143,7 +143,6 @@ const ScaleDistributionEditor: React.FC<FieldOverrideEditorProps<ScaleDistributi
/>
{value.type === ScaleDistribution.Log && (
<Select
menuShouldPortal
allowCustomValue={false}
options={LOG_DISTRIBUTION_OPTIONS}
value={value.log || 2}