diff --git a/superset-frontend/src/components/TimezoneSelector/index.tsx b/superset-frontend/src/components/TimezoneSelector/index.tsx index b63bf41eb537..b9ae8d65096b 100644 --- a/superset-frontend/src/components/TimezoneSelector/index.tsx +++ b/superset-frontend/src/components/TimezoneSelector/index.tsx @@ -19,8 +19,8 @@ import React, { useEffect, useRef } from 'react'; import moment from 'moment-timezone'; - -import { NativeGraySelect as Select } from 'src/components/Select'; +import { t } from '@superset-ui/core'; +import { Select } from 'src/components'; const DEFAULT_TIMEZONE = 'GMT Standard Time'; const MIN_SELECT_WIDTH = '375px'; @@ -92,12 +92,6 @@ const TIMEZONE_OPTIONS = TIMEZONES.sort( offsets: getOffsetKey(zone.name), })); -const timezoneOptions = TIMEZONE_OPTIONS.map(option => ( - - {option.label} - -)); - const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { const prevTimezone = useRef(timezone); const matchTimezoneToOptions = (timezone: string) => @@ -120,12 +114,12 @@ const TimezoneSelector = ({ onTimezoneChange, timezone }: TimezoneProps) => { return ( - {timezoneOptions} - + options={TIMEZONE_OPTIONS} + /> ); }; diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx index 8575a526a375..1509bff49292 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.test.jsx @@ -24,7 +24,7 @@ import fetchMock from 'fetch-mock'; import { act } from 'react-dom/test-utils'; import AlertReportModal from 'src/views/CRUD/alert/AlertReportModal'; import Modal from 'src/components/Modal'; -import { AsyncSelect, NativeGraySelect as Select } from 'src/components/Select'; +import { Select } from 'src/components'; import { Switch } from 'src/components/Switch'; import { Radio } from 'src/components/Radio'; import TextAreaControl from 'src/explore/components/controls/TextAreaControl'; @@ -161,11 +161,15 @@ describe('AlertReportModal', () => { }; const editWrapper = await mountAndWait(props); - expect(editWrapper.find(AsyncSelect).at(1).props().value).toEqual({ + expect( + editWrapper.find('[aria-label="Database"]').at(0).props().value, + ).toEqual({ value: 1, label: 'test database', }); - expect(editWrapper.find(AsyncSelect).at(2).props().value).toEqual({ + expect( + editWrapper.find('[aria-label="Chart"]').at(0).props().value, + ).toEqual({ value: 1, label: 'test chart', }); @@ -176,21 +180,9 @@ describe('AlertReportModal', () => { expect(wrapper.find('input[name="name"]')).toExist(); }); - it('renders three async select elements when in report mode', () => { - expect(wrapper.find(AsyncSelect)).toExist(); - expect(wrapper.find(AsyncSelect)).toHaveLength(3); - }); - - it('renders four async select elements when in alert mode', async () => { - const props = { - ...mockedProps, - isReport: false, - }; - - const addWrapper = await mountAndWait(props); - - expect(addWrapper.find(AsyncSelect)).toExist(); - expect(addWrapper.find(AsyncSelect)).toHaveLength(4); + it('renders five select elements when in report mode', () => { + expect(wrapper.find(Select)).toExist(); + expect(wrapper.find(Select)).toHaveLength(5); }); it('renders Switch element', () => { @@ -226,12 +218,12 @@ describe('AlertReportModal', () => { expect(input.props().value).toEqual('SELECT NaN'); }); - it('renders two select element when in report mode', () => { + it('renders five select element when in report mode', () => { expect(wrapper.find(Select)).toExist(); - expect(wrapper.find(Select)).toHaveLength(2); + expect(wrapper.find(Select)).toHaveLength(5); }); - it('renders three select elements when in alert mode', async () => { + it('renders seven select elements when in alert mode', async () => { const props = { ...mockedProps, isReport: false, @@ -240,7 +232,7 @@ describe('AlertReportModal', () => { const addWrapper = await mountAndWait(props); expect(addWrapper.find(Select)).toExist(); - expect(addWrapper.find(Select)).toHaveLength(3); + expect(addWrapper.find(Select)).toHaveLength(7); }); it('renders value input element when in alert mode', async () => { diff --git a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx index c39783ebbeaa..a16662d6e1a3 100644 --- a/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx +++ b/superset-frontend/src/views/CRUD/alert/AlertReportModal.tsx @@ -16,7 +16,13 @@ * specific language governing permissions and limitations * under the License. */ -import React, { FunctionComponent, useState, useEffect } from 'react'; +import React, { + FunctionComponent, + useState, + useEffect, + useMemo, + useCallback, +} from 'react'; import { styled, t, @@ -32,7 +38,7 @@ import { Switch } from 'src/components/Switch'; import Modal from 'src/components/Modal'; import TimezoneSelector from 'src/components/TimezoneSelector'; import { Radio } from 'src/components/Radio'; -import { AsyncSelect, NativeGraySelect as Select } from 'src/components/Select'; +import { Select } from 'src/components'; import { FeatureFlag, isFeatureEnabled } from 'src/featureFlags'; import withToasts from 'src/messageToasts/enhancers/withToasts'; import Owner from 'src/types/Owner'; @@ -215,10 +221,6 @@ const StyledSectionContainer = styled.div` } } } - - .hide-dropdown { - display: none; - } `; const StyledSectionTitle = styled.div` @@ -248,7 +250,7 @@ const StyledSwitchContainer = styled.div` `; export const StyledInputContainer = styled.div` - flex: 1 1 auto; + flex: 1; margin: ${({ theme }) => theme.gridUnit * 2}px; margin-top: 0; @@ -284,9 +286,7 @@ export const StyledInputContainer = styled.div` } input, - textarea, - .Select, - .ant-select { + textarea { flex: 1 1 auto; } @@ -300,36 +300,24 @@ export const StyledInputContainer = styled.div` } input::placeholder, - textarea::placeholder, - .Select__placeholder { + textarea::placeholder { color: ${({ theme }) => theme.colors.grayscale.light1}; } textarea, input[type='text'], - input[type='number'], - .Select__control, - .ant-select-single .ant-select-selector { - padding: ${({ theme }) => theme.gridUnit * 1.5}px + input[type='number'] { + padding: ${({ theme }) => theme.gridUnit}px ${({ theme }) => theme.gridUnit * 2}px; border-style: none; border: 1px solid ${({ theme }) => theme.colors.grayscale.light2}; border-radius: ${({ theme }) => theme.gridUnit}px; - .ant-select-selection-placeholder, - .ant-select-selection-item { - line-height: 24px; - } - &[name='description'] { flex: 1 1 auto; } } - .Select__control { - padding: 2px 0; - } - .input-label { margin-left: 10px; } @@ -581,94 +569,100 @@ const AlertReportModal: FunctionComponent = ({ }; // Fetch data to populate form dropdowns - const loadOwnerOptions = (input = '') => { - const query = rison.encode({ filter: input, page_size: SELECT_PAGE_SIZE }); - return SupersetClient.get({ - endpoint: `/api/v1/report/related/owners?q=${query}`, - }).then( - response => - response.json.result.map((item: any) => ({ - value: item.value, - label: item.text, - })), - badResponse => [], - ); - }; - - const loadSourceOptions = (input = '') => { - const query = rison.encode({ filter: input, page_size: SELECT_PAGE_SIZE }); - return SupersetClient.get({ - endpoint: `/api/v1/report/related/database?q=${query}`, - }).then( - response => { - const list = response.json.result.map((item: any) => ({ - value: item.value, - label: item.text, - })); - - setSourceOptions(list); - - // Find source if current alert has one set - if ( - currentAlert && - currentAlert.database && - !currentAlert.database.label - ) { - updateAlertState('database', getSourceData()); - } + const loadOwnerOptions = useMemo( + () => (input = '', page: number, pageSize: number) => { + const query = rison.encode({ filter: input, page, page_size: pageSize }); + return SupersetClient.get({ + endpoint: `/api/v1/report/related/owners?q=${query}`, + }).then(response => ({ + data: response.json.result.map( + (item: { value: number; text: string }) => ({ + value: item.value, + label: item.text, + }), + ), + totalCount: response.json.count, + })); + }, + [], + ); - return list; - }, - badResponse => [], - ); - }; + const getSourceData = useCallback( + (db?: MetaObject) => { + const database = db || currentAlert?.database; - const getSourceData = (db?: MetaObject) => { - const database = db || currentAlert?.database; + if (!database || database.label) { + return null; + } - if (!database || database.label) { - return null; - } + let result; - let result; + // Cycle through source options to find the selected option + sourceOptions.forEach(source => { + if (source.value === database.value || source.value === database.id) { + result = source; + } + }); - // Cycle through source options to find the selected option - sourceOptions.forEach(source => { - if (source.value === database.value || source.value === database.id) { - result = source; - } - }); + return result; + }, + [currentAlert?.database, sourceOptions], + ); - return result; + // Updating alert/report state + const updateAlertState = (name: string, value: any) => { + setCurrentAlert(currentAlertData => ({ + ...currentAlertData, + [name]: value, + })); }; - const loadDashboardOptions = (input = '') => { - const query = rison.encode({ filter: input, page_size: SELECT_PAGE_SIZE }); - return SupersetClient.get({ - endpoint: `/api/v1/report/related/dashboard?q=${query}`, - }).then( - response => { - const list = response.json.result.map((item: any) => ({ - value: item.value, - label: item.text, - })); + const loadSourceOptions = useMemo( + () => (input = '', page: number, pageSize: number) => { + const query = rison.encode({ filter: input, page, page_size: pageSize }); + return SupersetClient.get({ + endpoint: `/api/v1/report/related/database?q=${query}`, + }).then(response => { + const list = response.json.result.map( + (item: { value: number; text: string }) => ({ + value: item.value, + label: item.text, + }), + ); + setSourceOptions(list); + return { data: list, totalCount: response.json.count }; + }); + }, + [], + ); + const databaseLabel = + currentAlert && currentAlert.database && !currentAlert.database.label; + useEffect(() => { + // Find source if current alert has one set + if (databaseLabel) { + updateAlertState('database', getSourceData()); + } + }, [databaseLabel, getSourceData]); + + const loadDashboardOptions = useMemo( + () => (input = '', page: number, pageSize: number) => { + const query = rison.encode({ filter: input, page, page_size: pageSize }); + return SupersetClient.get({ + endpoint: `/api/v1/report/related/dashboard?q=${query}`, + }).then(response => { + const list = response.json.result.map( + (item: { value: number; text: string }) => ({ + value: item.value, + label: item.text, + }), + ); setDashboardOptions(list); - - // Find source if current alert has one set - if ( - currentAlert && - currentAlert.dashboard && - !currentAlert.dashboard.label - ) { - updateAlertState('dashboard', getDashboardData()); - } - - return list; - }, - badResponse => [], - ); - }; + return { data: list, totalCount: response.json.count }; + }); + }, + [], + ); const getDashboardData = (db?: MetaObject) => { const dashboard = db || currentAlert?.dashboard; @@ -689,62 +683,62 @@ const AlertReportModal: FunctionComponent = ({ return result; }; - const loadChartOptions = (input = '') => { - const query = rison.encode({ filter: input, page_size: SELECT_PAGE_SIZE }); - return SupersetClient.get({ - endpoint: `/api/v1/report/related/chart?q=${query}`, - }).then( - response => { - const list = response.json.result.map((item: any) => ({ - value: item.value, - label: item.text, - })); + const getChartData = useCallback( + (chartData?: MetaObject) => { + const chart = chartData || currentAlert?.chart; - setChartOptions(list); + if (!chart || chart.label) { + return null; + } - // Find source if current alert has one set - if (currentAlert && currentAlert.chart && !currentAlert.chart.label) { - updateAlertState('chart', getChartData()); - } + let result; - return list; - }, - badResponse => [], - ); - }; + // Cycle through chart options to find the selected option + chartOptions.forEach(slice => { + if (slice.value === chart.value || slice.value === chart.id) { + result = slice; + } + }); - const getChartData = (chartData?: MetaObject) => { - const chart = chartData || currentAlert?.chart; + return result; + }, + [chartOptions, currentAlert?.chart], + ); - if (!chart || chart.label) { - return null; + const noChartLabel = + currentAlert && currentAlert.chart && !currentAlert.chart.label; + useEffect(() => { + // Find source if current alert has one set + if (noChartLabel) { + updateAlertState('chart', getChartData()); } + }, [getChartData, noChartLabel]); + + const loadChartOptions = useMemo( + () => (input = '', page: number, pageSize: number) => { + const query = rison.encode({ filter: input, page, page_size: pageSize }); + return SupersetClient.get({ + endpoint: `/api/v1/report/related/chart?q=${query}`, + }).then(response => { + const list = response.json.result.map( + (item: { value: number; text: string }) => ({ + value: item.value, + label: item.text, + }), + ); - let result; - - // Cycle through chart options to find the selected option - chartOptions.forEach(slice => { - if (slice.value === chart.value || slice.value === chart.id) { - result = slice; - } - }); - - return result; - }; + setChartOptions(list); + return { data: list, totalCount: response.json.count }; + }); + }, + [], + ); const getChartVisualizationType = (chart: SelectValue) => SupersetClient.get({ endpoint: `/api/v1/chart/${chart.value}`, }).then(response => setChartVizType(response.json.result.viz_type)); - // Updating alert/report state - const updateAlertState = (name: string, value: any) => { - setCurrentAlert(currentAlertData => ({ - ...currentAlertData, - [name]: value, - })); - }; - // Handle input/textarea updates const onTextChange = ( event: React.ChangeEvent, @@ -775,11 +769,11 @@ const AlertReportModal: FunctionComponent = ({ updateAlertState('sql', value || ''); }; - const onOwnersChange = (value: Array) => { + const onOwnersChange = (value: Array) => { updateAlertState('owners', value || []); }; - const onSourceChange = (value: Array) => { + const onSourceChange = (value: Array) => { updateAlertState('database', value || []); }; @@ -832,8 +826,8 @@ const AlertReportModal: FunctionComponent = ({ const onContentTypeChange = (event: any) => { const { target } = event; - - setContentType(target.value); + // Gives time to close the select before changing the type + setTimeout(() => setContentType(target.value), 200); }; const onFormatChange = (event: any) => { @@ -1004,19 +998,6 @@ const AlertReportModal: FunctionComponent = ({ setIsHidden(false); } - // Dropdown options - const conditionOptions = CONDITIONS.map(condition => ( - - {condition.label} - - )); - - const retentionOptions = RETENTION_OPTIONS.map(option => ( - - {option.label} - - )); - return ( = ({ * - @@ -1107,19 +1097,23 @@ const AlertReportModal: FunctionComponent = ({ * - @@ -1148,21 +1142,14 @@ const AlertReportModal: FunctionComponent = ({ - {conditionOptions} - + options={CONDITIONS} + /> @@ -1224,21 +1211,12 @@ const AlertReportModal: FunctionComponent = ({ - {retentionOptions} - + value={currentAlert?.log_retention || DEFAULT_RETENTION} + options={RETENTION_OPTIONS} + /> @@ -1284,44 +1262,46 @@ const AlertReportModal: FunctionComponent = ({ {t('Dashboard')} {t('Chart')} - - {formatOptionEnabled && ( diff --git a/superset-frontend/src/views/CRUD/alert/components/NotificationMethod.tsx b/superset-frontend/src/views/CRUD/alert/components/NotificationMethod.tsx index 0e4b1b7e09be..905e85e7e547 100644 --- a/superset-frontend/src/views/CRUD/alert/components/NotificationMethod.tsx +++ b/superset-frontend/src/views/CRUD/alert/components/NotificationMethod.tsx @@ -18,7 +18,7 @@ */ import React, { FunctionComponent, useState } from 'react'; import { styled, t, useTheme } from '@superset-ui/core'; -import { NativeGraySelect as Select } from 'src/components/Select'; +import { Select } from 'src/components'; import Icons from 'src/components/Icons'; import { StyledInputContainer } from '../AlertReportModal'; @@ -116,26 +116,22 @@ export const NotificationMethod: FunctionComponent = ({ setRecipientValue(recipients); } - const methodOptions = (options || []).map((method: NotificationMethod) => ( - - {t(method)} - - )); - return ( ({ + label: method, + value: method, + }))} value={method} - > - {methodOptions} - + /> {method !== undefined && !!onRemove ? (