korbit-ai[bot] commented on code in PR #33826:
URL: https://github.com/apache/superset/pull/33826#discussion_r2156709727
##########
superset-frontend/src/features/databases/DatabaseModal/index.tsx:
##########
@@ -743,7 +749,8 @@ const DatabaseModal: FunctionComponent<DatabaseModalProps>
= ({
const handleClearValidationErrors = useCallback(() => {
setValidationErrors(null);
- }, [setValidationErrors]);
+ setHasValidated(false);
+ }, [setValidationErrors, setHasValidated]);
Review Comment:
### Overly Aggressive Validation Reset <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Clearing validation errors also resets the hasValidated flag, which forces
users to revalidate even for minor form changes.
###### Why this matters
Users have to revalidate the entire form after any change, even when
modifying unrelated fields, leading to a poor user experience.
###### Suggested change ∙ *Feature Preview*
Only clear validation errors without resetting validation state:
```typescript
const handleClearValidationErrors = useCallback(() => {
setValidationErrors(null);
}, [setValidationErrors]);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b95d26b-4a96-4a99-9e5c-564fb36b4b1e/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b95d26b-4a96-4a99-9e5c-564fb36b4b1e?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b95d26b-4a96-4a99-9e5c-564fb36b4b1e?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b95d26b-4a96-4a99-9e5c-564fb36b4b1e?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2b95d26b-4a96-4a99-9e5c-564fb36b4b1e)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:801aaee8-a4d5-435f-b3a1-f6dd54971e58 -->
[](801aaee8-a4d5-435f-b3a1-f6dd54971e58)
##########
superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx:
##########
@@ -112,63 +83,71 @@ const LabeledErrorBoundInput = ({
visibilityToggle,
get_url,
description,
+ isValidating = false,
...props
-}: LabeledErrorBoundInputProps) => (
- <StyledFormGroup className={className}>
- <StyledAlignment>
- <StyledFormLabel htmlFor={id} required={required}>
- {label}
- </StyledFormLabel>
- {hasTooltip && <InfoTooltip tooltip={`${tooltipText}`} />}
- </StyledAlignment>
- <FormItem
- css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
- validateTrigger={Object.keys(validationMethods)}
- validateStatus={errorMessage ? 'error' : 'success'}
- help={errorMessage || helpText}
- hasFeedback={!!errorMessage}
- >
- {visibilityToggle || props.name === 'password' ? (
- <StyledInputPassword
- {...props}
- {...validationMethods}
- iconRender={visible =>
- visible ? (
- <Tooltip title={t('Hide password.')}>
- <Icons.EyeInvisibleOutlined iconSize="m" css={iconReset} />
- </Tooltip>
- ) : (
- <Tooltip title={t('Show password.')}>
- <Icons.EyeOutlined
- iconSize="m"
- css={iconReset}
- data-test="icon-eye"
- />
- </Tooltip>
- )
- }
- role="textbox"
- />
- ) : (
- <StyledInput {...props} {...validationMethods} />
- )}
- {get_url && description ? (
- <Button
- type="link"
- htmlType="button"
- buttonStyle="default"
- onClick={() => {
- window.open(get_url);
- return true;
- }}
- >
- Get {description}
- </Button>
- ) : (
- <br />
- )}
- </FormItem>
- </StyledFormGroup>
-);
-
+}: LabeledErrorBoundInputProps) => {
+ const hasError = !!errorMessage;
+ const hasBeenValidated =
+ 'onBlur' in validationMethods && !!validationMethods.onBlur;
Review Comment:
### Incomplete Validation Method Check <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The validation state check only considers onBlur events, ignoring onChange
validation methods.
###### Why this matters
Fields using onChange validation will never show a success state, even when
valid, because the validation check is incomplete.
###### Suggested change ∙ *Feature Preview*
Include both validation methods in the check:
```typescript
const hasBeenValidated =
('onBlur' in validationMethods && !!validationMethods.onBlur) ||
('onChange' in validationMethods && !!validationMethods.onChange);
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e36f99f9-fc77-4bb3-afae-431ebf40c11f/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e36f99f9-fc77-4bb3-afae-431ebf40c11f?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e36f99f9-fc77-4bb3-afae-431ebf40c11f?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e36f99f9-fc77-4bb3-afae-431ebf40c11f?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e36f99f9-fc77-4bb3-afae-431ebf40c11f)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:6ca9246d-8421-40e6-924c-37de8dcaf00a -->
[](6ca9246d-8421-40e6-924c-37de8dcaf00a)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
