Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Check if actual checkbox value equals checkedValue #144

Merged
merged 2 commits into from
Mar 25, 2025
Merged

Check if actual checkbox value equals checkedValue #144

merged 2 commits into from
Mar 25, 2025

Conversation

jrauh01
Copy link
Contributor

@jrauh01 jrauh01 commented Mar 24, 2025 •
edited
Loading

Override method hasValue() from BaseFormElement in CheckboxElement.

Fix #143

@cla-bot cla-bot bot added the cla/signed label Mar 24, 2025
@jrauh01 jrauh01 force-pushed the fix/required-checkbox-validation branch from 57b0b19 to 7b6f673 Compare March 24, 2025 10:12
@jrauh01 jrauh01 requested a review from nilmerg March 24, 2025 10:34
nilmerg
nilmerg requested changes Mar 24, 2025
View reviewed changes
Copy link
Member

@nilmerg nilmerg left a comment

Choose a reason for hiding this comment

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

One thing to note is that an unchecked checkbox is now not validated anymore. But a quick search didn't yield any usage of an validator for checkboxes…

But, there's a test for this element, please add one covering this change.

@nilmerg nilmerg added this to the v0.9.0 milestone Mar 24, 2025
@nilmerg nilmerg added the bug Something isn't working label Mar 24, 2025
@jrauh01 jrauh01 force-pushed the fix/required-checkbox-validation branch from 140400d to 7d1feea Compare March 24, 2025 13:35
@jrauh01 jrauh01 force-pushed the fix/required-checkbox-validation branch 2 times, most recently from b8f1cdf to 1222b5e Compare March 24, 2025 13:44
@jrauh01
Copy link
Contributor Author

jrauh01 commented Mar 24, 2025

@nilmerg I think the test should now test the case correctly.

@jrauh01 jrauh01 requested a review from nilmerg March 24, 2025 13:46
@jrauh01 jrauh01 force-pushed the fix/required-checkbox-validation branch from 1222b5e to f37d36d Compare March 25, 2025 09:22
nilmerg
nilmerg previously approved these changes Mar 25, 2025
View reviewed changes
@jrauh01 jrauh01 force-pushed the fix/required-checkbox-validation branch from f37d36d to e9eb991 Compare March 25, 2025 09:53
@nilmerg nilmerg merged commit ba79288 into main Mar 25, 2025
35 of 36 checks passed
@nilmerg nilmerg deleted the fix/required-checkbox-validation branch March 25, 2025 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nilmerg nilmerg nilmerg approved these changes

Assignees

@jrauh01 jrauh01

Labels
bug Something isn't working cla/signed
Projects
None yet
Milestone
v0.9.0
Development

Successfully merging this pull request may close these issues.

Required checkboxes are not validated correctly on the server side
2 participants
@jrauh01 @nilmerg