Skip to content

Checkbox - add validation state invoked by validate() #2672

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 21 commits into from
Nov 2, 2023

Conversation

Inbal-Tish
Copy link
Collaborator

Description

Checkbox - add validation state invoked by validate() - see example in screen

Changelog

Checkbox - add validation state invoked by validate()

Additional info

#3662

@M-i-k-e-l
Copy link
Collaborator

If I add the following code and press it before doing anything I'll receive a valid state which is incorrect.

<Button
  size={'small'}
  label={`Is valid: ${this.state.isValid}`}
  onPress={() => this.setState({isValid: this.checkbox.current?.isValid()})}
/>

@Inbal-Tish
Copy link
Collaborator Author

If I add the following code and press it before doing anything I'll receive a valid state which is incorrect.

<Button
  size={'small'}
  label={`Is valid: ${this.state.isValid}`}
  onPress={() => this.setState({isValid: this.checkbox.current?.isValid()})}
/>

Why? The component is valid by default until it moves to the validation state, which means the user invoked the validate() method and it passed/failed the validation check. If the user doesn't invoke the validate() I will not show error colors if the component is unchecked and the component will keep acting as valid (as it always did)

@Inbal-Tish Inbal-Tish requested a review from M-i-k-e-l August 2, 2023 09:16
@Inbal-Tish Inbal-Tish requested a review from M-i-k-e-l August 14, 2023 08:16
@stale
Copy link

stale bot commented Oct 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 15, 2023
@M-i-k-e-l M-i-k-e-l removed the wontfix label Oct 15, 2023
…/Checkbox_error_state

# Conflicts:
#	demo/src/screens/componentScreens/CheckboxScreen.tsx
//eslint-disable-next-line
new CheckboxDriver({component, testID});

checkboxRef.current?.validate();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you add a ? you don't know that validate was actually called (what if current is undefined?

@@ -86,8 +94,17 @@ export interface CheckboxProps extends TouchableOpacityProps {
indeterminate?: boolean;
}

interface CheckboxMethods {
validate: () => void;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see in TextField there's a onValidationFailed method that is called when the validation fails, WDYT about adding it? Unlike onChangeValidity it is called if validate fails.
It can be another PR...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll keep it simple until we'll get such a request

@Inbal-Tish Inbal-Tish merged commit 6ed014a into master Nov 2, 2023
@ethanshar
Copy link
Collaborator

@Inbal-Tish
This PR breaks tests in private.
Can you please take a look.
You will need to update snapshot version in private using yarn up react-native-ui-lib@snapshot

@Inbal-Tish Inbal-Tish deleted the feat/Checkbox_error_state branch December 7, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants