Create a5 d2 UI - Githubissues

mlan225 commented 3 months ago

Resolves: #133

[x] Create form Ui by adding questions, labels, formatting, and other text
[x] Make sure the save functionality works for complete, partial save, etc.
[x] Add behaviors for enable/disable
[x] Add simple validation to the model
[x] Add any complex validation that may be required
[x] Review the form Ui to clean up any issues with the front end (run spell check, double check element structure, etc.)
[x] Apply edits made to the a5d2 NACC repo
[x] Add custom validation that we talked about for C2

mlan225 commented 3 months ago

All questions have been added to the form. Moving on to make sure backend save and edit functionality works as intended

ashleybot commented 3 months ago

@mlan225 Saw some potential changes for A5D2 => https://github.com/naccdata/uniform-data-set/pull/147/files

ashleybot commented 3 months ago

Screen Shot 2024-03-25 at 1 16 37 PM

mlan225 commented 3 months ago

@ashleybot One more variable that may need a text adjustment. Posting up the evidence OTHERCOND in the documentation OTHCOND in the API

NACC documentation

API model

ashleybot commented 3 months ago

@ashleybot One more variable that may need a text adjustment. Posting up the evidence OTHERCOND in the documentation OTHCOND in the API

@mlan225 Looks like that variable name was changed along with CANCERACTV on Feb. 29 in this commit => https://github.com/naccdata/uniform-data-set/commit/633d6b6d73301ea8495c4045ed14bde4e0248fcf

Making the hotfix now and I'll let you know when it's ready.

ashleybot commented 3 months ago

Hotfix pushed to uniform-data-set-dotnet-api with migration v2.0.4

mlan225 commented 3 months ago

Similar to the A1a, I made the A5D2 is available on IVP and TIP visits

mlan225 commented 3 months ago

uh.. huh? lol

mlan225 commented 3 months ago

Im going to convert back to draft and move out of review until we can take a look at this linter error

mlan225 commented 3 months ago

Went in a a bit of a rabbit hole and couldn't get the .gitleaksignore file to ignore the fingerprint of the error: https://github.com/gitleaks/gitleaks?tab=readme-ov-file#gitleaksignore

Undid any changes to the PR related to testing this

mlan225 commented 3 months ago

@ashleybot Ready for review

Not super sure what we should do about the gitleaks error yet, I wasn't able to get the ignore settings file to work for the action run. I'd assume this error will persist into main if we don't do anything about it here but I also don't want it to hold up a review of the form itself in the meantime.

Review checklist:

[ ] A5D2 form has been added
[ ] A5D2 can be partially saved / completely saved
[ ] A5D2 validation functions correctly
[ ] A5D2 client-side validation behaviors enable and disable fields successfully
[ ] Text labels, section headers, and instruction text match the NACC documentation
[ ] Modification to table cell tag helper does not break existing functionality

mlan225 commented 3 months ago

Will update the release version after the review process

ashleybot commented 3 months ago

@smiththay Can you review this?

smiththay commented 2 months ago

Marvin and I paired and removed Tailwind dark theme class modifier. Continuing the review.

smiththay commented 2 months ago

Hey @mlan225, noticed a few behavior items not functioning correctly.

1.) For question 4g. the checkboxes in 4g3. and question 4g4. do not get disabled when 4g. is Unknown Screenshot 2024-04-08 at 4 53 56 PM

2.) Similar situation is happening here for question 5f. where checkboxes are not disabled if 5f. is absent or unknown. Screenshot 2024-04-08 at 3 43 03 PM

3.) For questions 5m. the checkboxes are disabled but 5m11. remains enabled. Screenshot 2024-04-08 at 3 47 56 PM Screenshot 2024-04-08 at 3 48 19 PM

4.)For the checkboxes that are disabled, they visually appear enabled like in question 5a. This can be resolved by adding disabled:bg-slate-50 disabled:text-slate-500 disabled:border-slate-200 disabled:shadow-none to the checkboxes class. Screenshot 2024-04-08 at 3 41 22 PM

ashleybot commented 2 months ago

@mlan225 It looks like the PDF was also updated last week. It's a good time to take a look at the copy to see if there are any updates. https://github.com/naccdata/uniform-data-set/pull/152#pullrequestreview-1972332503

[x] Compare the copy in the PDF https://github.com/naccdata/uniform-data-set/blob/main/forms/uds/a5d2/UDS4-IVP-Form-A5-D2-FinalProof.pdf

mlan225 commented 2 months ago

Applied your PR comments, @smiththay you mind running this and giving it a quick look at the changed stuff to verify?

The recent commits:

Resolved the PR review comments
Updated text with updates to the PDF
fixed some minor issues with the subcounters with sub-sections

ashleybot commented 2 months ago

Confirmed please check out some quick changes @smiththay

smiththay commented 2 months ago

@mlan225 This form looks great, especially with how massive and involved it it. There are just a few more small items I discovered on the additional review. 1.) CANCOTHERX does not have parameters for Maxlength and it is throwing error. Screenshot 2024-04-22 at 3 01 21 PM 2.) CANCERAGE does not have any validation so I can save the form without input Screenshot 2024-04-22 at 3 05 36 PM Screenshot 2024-04-22 at 3 02 42 PM 3.) HIVAGE has a range of 0-2 or 9 currently but the docs state 0-110, 999 Screenshot 2024-04-22 at 3 04 19 PM Screenshot 2024-04-22 at 3 03 32 PM