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.

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

[DUOS-2635] Implemented disabled checkboxes for data submission form #2329

Merged
merged 1 commit into from
Sep 12, 2023

Conversation

hams7504
Copy link
Contributor

Addresses

https://broadworkbench.atlassian.net/browse/DUOS-2635

Summary

  • Added a non-selection cursor to the checkboxes in the Data Submission Form when the consent group is prefilled (when the checkboxes are disabled)

Have you read Terra's Contributing Guide lately? If not, do that first.

  • Label PR with a Jira ticket number and include a link to the ticket
  • Label PR with a security risk modifier [no, low, medium, high]
  • PR describes scope of changes
  • Get a minimum of one thumbs worth of review, preferably two if enough team members are available
  • Get PO sign-off for all non-trivial UI or workflow changes
  • Verify all tests go green
  • Test this change deployed correctly and works on dev environment after deployment

@hams7504 hams7504 requested a review from a team as a code owner September 11, 2023 20:16
@hams7504 hams7504 requested a review from rushtong September 11, 2023 20:16
Copy link
Contributor

@rushtong rushtong left a comment

Choose a reason for hiding this comment

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

Looks good! 👍🏽

Comment on lines 485 to 490
label({
className: `regular-checkbox ${!isValid(validation) ? 'errored' : ''}`,
htmlFor: `${id}`,
style: disabled ? { cursor: 'not-allowed' } : null
}, [toggleText])
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm seeing two things unrelated to the ticket that shouldn't hold up this PR, maybe we can address separately.

  1. I had the question of Why is the input not inside the label? and needed to look that up. I learned HTML in the 90's, but apparently it can be before, around, or after
  2. We're still using hyperscript here ... would be really nice to get rid of that and stick with jsx.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Yes! And I also don't understand why the input tag is not directly creating the checkbox.
  2. Definitely agree!

Copy link
Contributor

@nmalfroy nmalfroy left a comment

Choose a reason for hiding this comment

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

Looks good! Might just be worth double checking the behavior if it's enabled (I'm assuming it's fine)

@@ -485,6 +485,7 @@ export const FormInputCheckbox = (config) => {
label({
className: `regular-checkbox ${!isValid(validation) ? 'errored' : ''}`,
htmlFor: `${id}`,
style: disabled ? { cursor: 'not-allowed' } : null
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm assuming that this will default to pointer in the null case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's right!

@hams7504 hams7504 merged commit 1f67736 into develop Sep 12, 2023
@hams7504 hams7504 deleted the hm-duos-2635-disable-checkbox branch September 12, 2023 15:12
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