Skip to content
This repository was archived by the owner on Feb 21, 2023. It is now read-only.

Conversation

reeseschultz
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.8%) to 93.665% when pulling f9b1870 on feature/checkbox-list into fc41248 on master.

@TheSharpieOne
Copy link
Collaborator

Styling issue when the grouped checkboxes are invalid
image

Copy link
Collaborator

@TheSharpieOne TheSharpieOne left a comment

Choose a reason for hiding this comment

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

There is a styling issue when the checkbox groups are invalid.

Also, can you alias minChecked and maxChecked to be min and max as props on the AvCheckboxGroup (or just modify the existing min/max validations to work with arrays) to allow for something like:

<AvCheckboxGroup name="checkboxExample" label="U Pick 2!" max="2" min="2" required>
  <AvCheckbox label="Bulbasaur" value="Bulbasaur" />
  <AvCheckbox label="Squirtle" value="Squirtle" />
  <AvCheckbox label="Charmander" value="Charmander" />
  <AvCheckbox label="Pikachu" value="Pikachu" disabled />
</AvCheckboxGroup>

@robmcguinness
Copy link
Contributor

@TheSharpieOne good catch!

@reeseschultz
Copy link
Contributor Author

reeseschultz commented Dec 17, 2018

I will update the aliasing. Also, I'll change the styling.

@reeseschultz
Copy link
Contributor Author

@TheSharpieOne I pushed a couple commits to address the issues you mentioned. When you have time, please take another look and let me know what you think.

@netlify
Copy link

netlify bot commented Dec 19, 2018

Deploy preview for availity-reactstrap-validation ready!

Built with commit 6db1f46

https://deploy-preview-106--availity-reactstrap-validation.netlify.com

@TheSharpieOne
Copy link
Collaborator

The min and max checked validation also have the style issue. Not sure the best way to handle this as this is where it really starts to get different from radios.
The styles themselves are tied to the DOM's ValidityState. You could use .setCustomValidity on the underlying checkbox to control the ValidityState.
image
image

@reeseschultz
Copy link
Contributor Author

Thanks for your feedback. I'll dig into the code and come up with an appropriate fix.

@reeseschultz
Copy link
Contributor Author

@TheSharpieOne The .setCustomValidity function did the trick! Much appreciated.

Also, thanks for bearing with me. I just learned React for the sake of updating this library. Is there anything else you want me to add/change before the merge?

Copy link
Collaborator

@TheSharpieOne TheSharpieOne left a comment

Choose a reason for hiding this comment

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

LGTM

@robmcguinness robmcguinness merged commit ede0d1f into master Dec 20, 2018
@robmcguinness robmcguinness deleted the feature/checkbox-list branch December 26, 2018 12:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants