betodealmeida commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r634044554



##########
File path: 
superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx
##########
@@ -33,24 +33,24 @@ export const InteractiveLabeledErrorBoundInput = ({
   type,
   id,
 }: LabeledErrorBoundInputProps) => {
-  const [checkErrorMessage, setCheckErrorMessage] = useState('');
   const [currentValue, setCurrentValue] = useState(value);
 
-  const validateFunctionality: (value: any) => void = value => {
+  const validateFunctionality: (value: any) => string = value => {
     setCurrentValue(value.target.value);
     if (value.target.value.includes('success')) {
-      setCheckErrorMessage('');
-    } else {
-      setCheckErrorMessage('Type success in the text bar');
+      return 'success';
     }
+    return 'error';
   };

Review comment:
       My suggestion was slightly different, having no state at all in the 
validator:
   
   ```js
     const validateFunctionality: (value: any) => (string | void) = value => {
       if not (value.target.value.includes('success')) {
         return 'Type success in the text bar';
       }
     };
   ```
   
   Basically, if there's an error return an error message. If the validation is 
good, return nothing (void, null, undefined, etc.). The function has no 
side-effects and uses no external hooks.
   
   This way, we can implement reusable validators:
   
   ```js
   const isInteger: (value: any) => (string | void) = value => {
       const reg = new RegExp('^\\d+$');
       if not reg.match(value) {
           return 'Number is not an integer';
       }
   };
   ```
   
   Then, in the `LabeledErrorBoundInput` component we'd wrap the validators in 
a function that sets the error message. Something like (sorry for not using 
types for writing old-school JS):
   
   ```js
   const LabeledErrorBoundInput = ({
     label,
     validationMethods,
     //errorMessage,  // remove this, we'll read it from the validation methods
     helpText,
     required = false,
     id,
     ...props
   }: LabeledErrorBoundInputProps) => {
     const [errorMessage, setErrorMessage] = useState('');
   
     // wrap validators in a function that sets the error message
     const validators = {};
     for (const [trigger, validator] of Object.entries(validationMethods)) {
       validators[trigger] = function(value) {
         const message = validator(value);
         if (message) {
           setErrorMessage(message);
         }
       };
     }
   
     return (
       <>
         <FormLabel htmlFor={id} required={required}>
           {label}
         </FormLabel>
         <FormItem
           css={(theme: SupersetTheme) => alertIconStyles(theme, 
!!errorMessage)}
           validateTrigger={Object.keys(validators)}
           validateStatus={errorMessage ? 'error' : 'success'}
           help={errorMessage || helpText}
           hasFeedback={!!errorMessage}
         >
           <StyledInput {...props} {...validators} />
         </FormItem>
       </>
     )
   };
   ```
   
       




-- 
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to