fix(explore): conditional formatting value validators (#16230)

* fix(explore): conditional formatting value validators

* Fix typing, make validator more generic

* Remove commented code

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>
+);