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
Refactor: Eligibility start page #1498
Conversation
584b639
to
57f083e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
I tried editing the start.html
file (removing the whole help-text
section) to see how the includes would respond and it worked perfectly + quickly (the whole section should be removed, even if the includes calls for it). Sticking with the trusty includes has led to code that's easier for me to read - no more djLint on/off switches - and more plain HTML/regular includes block overrides.
One change request: I believe the secondary_details
code I wrote last Sprint can be deleted:
- remove parameter from MediaItem view model
- remove from
media-list.html
template
fdf8479
to
46f9283
Compare
Rebased on the changes in #1489 EDIT: as y'all were merging that one and approving this one 🤣 |
reference the template file to use for Eligibility Start make other start fields optional
46f9283
to
73243d8
Compare
eligibility/start template specifically for mst courtesy cards
73243d8
to
abed6aa
Compare
abed6aa
to
37bd314
Compare
37bd314
to
56261a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking really good so far!
Ready for re-review. I couldn't fully get rid of the tag work from #1488, but I tried to squash it down to a few logical commits for the structure, and one big refactor for the includes. Thanks for all the comments on the earlier PR, and the radio buttons PR, that all helped clarify a better way forward. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read through the code changes and run the app locally. This refactor looks great!! 💯
Supersedes #1488 -- ultimately moved away from Django template tags in favor of our trusty includes.
Stole some ideas from #1489
Part of #1442
Closes #1477
Goals
Overview
EligibilityVerifier
to reference a custom template for Start pageeligibility/start.html
eligibility/start__login_gov.html
eligibility/start__mst_courtesy_card.html
eligibility/start__veteran.html
eligibility:start
view to use the verifier's template