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

Fix: Checkbox component to work well with useForm #211

Merged
merged 4 commits into from
Jul 29, 2024
Merged

Fix: Checkbox component to work well with useForm #211

merged 4 commits into from
Jul 29, 2024

Conversation

jbouder
Copy link
Collaborator

@jbouder jbouder commented Jul 24, 2024 •
edited
Loading

Update USWDS Checkbox and Radio Button components to properly apply a default value when specified.

Description

  • Fixed checkbox default checked.
  • Fixed radio button default checked.
  • Added basic single checkbox and radio button stories.

Related Issue

#209

Motivation and Context

  • Bug fix

How Has This Been Tested?

  • Local Testing

Screenshots (if appropriate):

@jbouder jbouder added the bug Something isn't working label Jul 29, 2024
@jbouder jbouder marked this pull request as ready for review July 29, 2024 14:54
jbouder
jbouder commented Jul 29, 2024
View reviewed changes
packages/comet-uswds/src/components/checkbox/checkbox.tsx
@@ -65,8 +70,8 @@ export const Checkbox = ({
id={inputId}
type="checkbox"
name={name}
defaultValue={value}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not a valid attribute for a checkbox.

jbouder
jbouder commented Jul 29, 2024
View reviewed changes
packages/comet-uswds/src/components/radio-button/radio-button.tsx
@@ -65,8 +70,8 @@ export const RadioButton = ({
id={inputId}
type="radio"
name={name}
defaultValue={value}
Copy link
Collaborator Author

@jbouder jbouder Jul 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not a valid attribute for a radio.

kildre
kildre approved these changes Jul 29, 2024
View reviewed changes
Copy link
Collaborator

@kildre kildre left a comment

Choose a reason for hiding this comment

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

Only thing I noticed was 14 failed tests, but I'm sure you're aware of those, none had to do with these changes.

Looks good!

@jbouder
Copy link
Collaborator Author

jbouder commented Jul 29, 2024 •
edited by kildre
Loading

Only thing I noticed was 14 failed tests, but I'm sure you're aware of those, none had to do with these changes.

Looks good!

Where do you see failed tests? I don't see any in the test run here: https://github.com/MetroStar/comet/actions/runs/10146635791/job/28055157858?pr=211

image

When I run tests locally.

@jbouder
Copy link
Collaborator Author

jbouder commented Jul 29, 2024

Only thing I noticed was 14 failed tests, but I'm sure you're aware of those, none had to do with these changes.
Looks good!

Where do you see failed tests? I don't see any in the test run here: https://github.com/MetroStar/comet/actions/runs/10146635791/job/28055157858?pr=211

image When I run tests locally.

Hmmm....I'm not seeing that. And the automated tests look good. So i'm gonna say its working :D

@jbouder jbouder merged commit b73308a into main Jul 29, 2024
5 checks passed
@jbouder jbouder deleted the uswds-cbx-fix branch July 29, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kildre kildre kildre approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jbouder @kildre