[GitHub] [superset] betodealmeida commented on a change in pull request #14530: feat: Labeled Error-bound Input

[GitBox <gi...@apache.org>]

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 this pseudocode:
   
   ```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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org