This is an automated email from the ASF dual-hosted git repository. villebro pushed a commit to branch 1.3 in repository https://gitbox.apache.org/repos/asf/superset.git
commit 90f432e18c1ecdae507ca8018f4195f4319f8a6b Author: Kamil Gabryjelski <[email protected]> AuthorDate: Thu Aug 12 14:31:29 2021 +0200 fix(explore): conditional formatting value validators (#16230) * fix(explore): conditional formatting value validators * Fix typing, make validator more generic * Remove commented code (cherry picked from commit a16e2907655148c79860055fcec9e956392cb1ec) --- superset-frontend/src/components/Form/Form.tsx | 2 + superset-frontend/src/components/Form/index.tsx | 4 +- .../FormattingPopoverContent.tsx | 330 ++++++++++----------- 3 files changed, 168 insertions(+), 168 deletions(-) diff --git a/superset-frontend/src/components/Form/Form.tsx b/superset-frontend/src/components/Form/Form.tsx index 9463245..1fc25e6 100644 --- a/superset-frontend/src/components/Form/Form.tsx +++ b/superset-frontend/src/components/Form/Form.tsx @@ -32,3 +32,5 @@ const StyledForm = styled(AntDForm)` export default function Form(props: FormProps) { return <StyledForm {...props} />; } + +export { FormProps }; diff --git a/superset-frontend/src/components/Form/index.tsx b/superset-frontend/src/components/Form/index.tsx index 7d7a607..1aa62ae 100644 --- a/superset-frontend/src/components/Form/index.tsx +++ b/superset-frontend/src/components/Form/index.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import Form from './Form'; +import Form, { FormProps } from './Form'; import FormItem from './FormItem'; import FormLabel from './FormLabel'; import LabeledErrorBoundInput from './LabeledErrorBoundInput'; -export { Form, FormItem, FormLabel, LabeledErrorBoundInput }; +export { Form, FormItem, FormLabel, LabeledErrorBoundInput, FormProps }; diff --git a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx index b23b456..85f406c 100644 --- a/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx +++ b/superset-frontend/src/explore/components/controls/ConditionalFormattingControl/FormattingPopoverContent.tsx @@ -16,9 +16,9 @@ * specific language governing permissions and limitations * under the License. */ -import React, { useCallback, useMemo } from 'react'; +import React from 'react'; import { styled, t } from '@superset-ui/core'; -import { Form, FormItem } from 'src/components/Form'; +import { Form, FormItem, FormProps } from 'src/components/Form'; import { Select } from 'src/components'; import { Col, InputNumber, Row } from 'src/common/components'; import Button from 'src/components/Button'; @@ -57,6 +57,127 @@ const operatorOptions = [ { value: COMPARATOR.BETWEEN_OR_RIGHT_EQUAL, label: '< x ≤' }, ]; +const targetValueValidator = ( + compare: (targetValue: number, compareValue: number) => boolean, + rejectMessage: string, +) => (targetValue: number | string) => ( + _: any, + compareValue: number | string, +) => { + if ( + !targetValue || + !compareValue || + compare(Number(targetValue), Number(compareValue)) + ) { + return Promise.resolve(); + } + return Promise.reject(new Error(rejectMessage)); +}; + +const targetValueLeftValidator = targetValueValidator( + (target: number, val: number) => target > val, + t('This value should be smaller than the right target value'), +); + +const targetValueRightValidator = targetValueValidator( + (target: number, val: number) => target < val, + t('This value should be greater than the left target value'), +); + +const isOperatorMultiValue = (operator?: COMPARATOR) => + operator && MULTIPLE_VALUE_COMPARATORS.includes(operator); + +const isOperatorNone = (operator?: COMPARATOR) => + !operator || operator === COMPARATOR.NONE; + +const rulesRequired = [{ required: true, message: t('Required') }]; + +type GetFieldValue = Pick<Required<FormProps>['form'], 'getFieldValue'>; +const rulesTargetValueLeft = [ + { required: true, message: t('Required') }, + ({ getFieldValue }: GetFieldValue) => ({ + validator: targetValueLeftValidator(getFieldValue('targetValueRight')), + }), +]; + +const rulesTargetValueRight = [ + { required: true, message: t('Required') }, + ({ getFieldValue }: GetFieldValue) => ({ + validator: targetValueRightValidator(getFieldValue('targetValueLeft')), + }), +]; + +const targetValueLeftDeps = ['targetValueRight']; +const targetValueRightDeps = ['targetValueLeft']; + +const shouldFormItemUpdate = ( + prevValues: ConditionalFormattingConfig, + currentValues: ConditionalFormattingConfig, +) => + isOperatorNone(prevValues.operator) !== + isOperatorNone(currentValues.operator) || + isOperatorMultiValue(prevValues.operator) !== + isOperatorMultiValue(currentValues.operator); + +const operatorField = ( + <FormItem + name="operator" + label={t('Operator')} + rules={rulesRequired} + initialValue={operatorOptions[0].value} + > + <Select ariaLabel={t('Operator')} options={operatorOptions} /> + </FormItem> +); + +const renderOperatorFields = ({ getFieldValue }: GetFieldValue) => + isOperatorNone(getFieldValue('operator')) ? ( + <Row gutter={12}> + <Col span={6}>{operatorField}</Col> + </Row> + ) : isOperatorMultiValue(getFieldValue('operator')) ? ( + <Row gutter={12}> + <Col span={9}> + <FormItem + name="targetValueLeft" + label={t('Left value')} + rules={rulesTargetValueLeft} + dependencies={targetValueLeftDeps} + validateTrigger="onBlur" + trigger="onBlur" + > + <FullWidthInputNumber /> + </FormItem> + </Col> + <Col span={6}>{operatorField}</Col> + <Col span={9}> + <FormItem + name="targetValueRight" + label={t('Right value')} + rules={rulesTargetValueRight} + dependencies={targetValueRightDeps} + validateTrigger="onBlur" + trigger="onBlur" + > + <FullWidthInputNumber /> + </FormItem> + </Col> + </Row> + ) : ( + <Row gutter={12}> + <Col span={6}>{operatorField}</Col> + <Col span={18}> + <FormItem + name="targetValue" + label={t('Target value')} + rules={rulesRequired} + > + <FullWidthInputNumber /> + </FormItem> + </Col> + </Row> + ); + export const FormattingPopoverContent = ({ config, onChange, @@ -65,167 +186,44 @@ export const FormattingPopoverContent = ({ config?: ConditionalFormattingConfig; onChange: (config: ConditionalFormattingConfig) => void; columns: { label: string; value: string }[]; -}) => { - const isOperatorMultiValue = (operator?: COMPARATOR) => - operator && MULTIPLE_VALUE_COMPARATORS.includes(operator); - - const isOperatorNone = (operator?: COMPARATOR) => - !operator || operator === COMPARATOR.NONE; - - const operatorField = useMemo( - () => ( - <FormItem - name="operator" - label={t('Operator')} - rules={[{ required: true, message: t('Required') }]} - initialValue={operatorOptions[0].value} - > - <Select ariaLabel={t('Operator')} options={operatorOptions} /> - </FormItem> - ), - [], - ); - - const targetValueLeftValidator = useCallback( - (rightValue?: number) => (_: any, value?: number) => { - if (!value || !rightValue || rightValue > value) { - return Promise.resolve(); - } - return Promise.reject( - new Error( - t('This value should be smaller than the right target value'), - ), - ); - }, - [], - ); - - const targetValueRightValidator = useCallback( - (leftValue?: number) => (_: any, value?: number) => { - if (!value || !leftValue || leftValue < value) { - return Promise.resolve(); - } - return Promise.reject( - new Error(t('This value should be greater than the left target value')), - ); - }, - [], - ); - - return ( - <Form - onFinish={onChange} - initialValues={config} - requiredMark="optional" - layout="vertical" - > - <Row gutter={12}> - <Col span={12}> - <FormItem - name="column" - label={t('Column')} - rules={[{ required: true, message: t('Required') }]} - initialValue={columns[0]?.value} - > - <Select ariaLabel={t('Select column')} options={columns} /> - </FormItem> - </Col> - <Col span={12}> - <FormItem - name="colorScheme" - label={t('Color scheme')} - rules={[{ required: true, message: t('Required') }]} - initialValue={colorSchemeOptions[0].value} - > - <Select - ariaLabel={t('Color scheme')} - options={colorSchemeOptions} - /> - </FormItem> - </Col> - </Row> - <FormItem - noStyle - shouldUpdate={( - prevValues: ConditionalFormattingConfig, - currentValues: ConditionalFormattingConfig, - ) => - isOperatorNone(prevValues.operator) !== - isOperatorNone(currentValues.operator) || - isOperatorMultiValue(prevValues.operator) !== - isOperatorMultiValue(currentValues.operator) - } - > - {({ getFieldValue }) => - isOperatorNone(getFieldValue('operator')) ? ( - <Row gutter={12}> - <Col span={6}>{operatorField}</Col> - </Row> - ) : isOperatorMultiValue(getFieldValue('operator')) ? ( - <Row gutter={12}> - <Col span={9}> - <FormItem - name="targetValueLeft" - label={t('Left value')} - rules={[ - { required: true, message: t('Required') }, - ({ getFieldValue }) => ({ - validator: targetValueLeftValidator( - getFieldValue('targetValueRight'), - ), - }), - ]} - dependencies={['targetValueRight']} - validateTrigger="onBlur" - trigger="onBlur" - > - <FullWidthInputNumber /> - </FormItem> - </Col> - <Col span={6}>{operatorField}</Col> - <Col span={9}> - <FormItem - name="targetValueRight" - label={t('Right value')} - rules={[ - { required: true, message: t('Required') }, - ({ getFieldValue }) => ({ - validator: targetValueRightValidator( - getFieldValue('targetValueLeft'), - ), - }), - ]} - dependencies={['targetValueLeft']} - validateTrigger="onBlur" - trigger="onBlur" - > - <FullWidthInputNumber /> - </FormItem> - </Col> - </Row> - ) : ( - <Row gutter={12}> - <Col span={6}>{operatorField}</Col> - <Col span={18}> - <FormItem - name="targetValue" - label={t('Target value')} - rules={[{ required: true, message: t('Required') }]} - > - <FullWidthInputNumber /> - </FormItem> - </Col> - </Row> - ) - } - </FormItem> - <FormItem> - <JustifyEnd> - <Button htmlType="submit" buttonStyle="primary"> - {t('Apply')} - </Button> - </JustifyEnd> - </FormItem> - </Form> - ); -}; +}) => ( + <Form + onFinish={onChange} + initialValues={config} + requiredMark="optional" + layout="vertical" + > + <Row gutter={12}> + <Col span={12}> + <FormItem + name="column" + label={t('Column')} + rules={rulesRequired} + initialValue={columns[0]?.value} + > + <Select ariaLabel={t('Select column')} options={columns} /> + </FormItem> + </Col> + <Col span={12}> + <FormItem + name="colorScheme" + label={t('Color scheme')} + rules={rulesRequired} + initialValue={colorSchemeOptions[0].value} + > + <Select ariaLabel={t('Color scheme')} options={colorSchemeOptions} /> + </FormItem> + </Col> + </Row> + <FormItem noStyle shouldUpdate={shouldFormItemUpdate}> + {renderOperatorFields} + </FormItem> + <FormItem> + <JustifyEnd> + <Button htmlType="submit" buttonStyle="primary"> + {t('Apply')} + </Button> + </JustifyEnd> + </FormItem> + </Form> +);
