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

Complete sign and submit 187003764 #43

Merged
merged 12 commits into from
Feb 15, 2024

Conversation

vrajmohan
Copy link
Contributor

No description provided.

@vrajmohan vrajmohan force-pushed the complete-sign-and-submit-187003764 branch 2 times, most recently from a6a8ea6 to 13b90f0 Compare February 11, 2024 15:27
@bseeger bseeger self-requested a review February 12, 2024 15:13
@bseeger
Copy link
Contributor

bseeger commented Feb 12, 2024

Related to this PR. This line's fonts are smaller than the design. I'm wondering if you can try it as a subheader in the cardHeader font:

cardHeader(header=#{legal-stuff.header}, subheader=#{legal-stuff.help})

@bseeger
Copy link
Contributor

bseeger commented Feb 12, 2024

Related to this PR:
The wording on the http://localhost:8080/flow/mdBenefitsFlow/sensitiveConvictionQuestionsIntro is different from the design. The apps page has more text than the design has.

@bseeger
Copy link
Contributor

bseeger commented Feb 12, 2024

Also related to this PR:
In the "Your Right's" section on the rightsAndResponsibility page all the page links to external references are missing. The listed URLs are not <a href>s.

Under "TCA and SNAP Penalties" - the first line ("Do not:") should be bold.

@bseeger
Copy link
Contributor

bseeger commented Feb 12, 2024

the rightsAndResponsibilites page is missing the "Medicaid Warning and Penalties" section.

@bseeger
Copy link
Contributor

bseeger commented Feb 12, 2024

The first, second and signature page seems different from the design. Not sure what's changed. I know I listed things outside of this PR, but I thought this PR was completing this section and it looks like there is more work to be done. Separate ticket?

@vrajmohan vrajmohan force-pushed the complete-sign-and-submit-187003764 branch 6 times, most recently from 0201a07 to 9737a69 Compare February 14, 2024 23:40
Add a checkboxFieldsetUnescaped fragment to achieve this.
1. We were accidentally copying smart quotes from Figma
2. Had to style <ul> explicitly as Honeycrisp was overriding the "natural"
   styles
@vrajmohan vrajmohan force-pushed the complete-sign-and-submit-187003764 branch 2 times, most recently from 23b7138 to bf88fb7 Compare February 15, 2024 04:02
- Make bold to mimic the PDF
- Use subtext for form header help text
- Format external links
We decided to include this in T&Cs even though we are not supporting
Medicaid.
@vrajmohan vrajmohan force-pushed the complete-sign-and-submit-187003764 branch from bf88fb7 to b2e332a Compare February 15, 2024 14:17
Copy link
Contributor

@bseeger bseeger left a comment

Choose a reason for hiding this comment

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

I have one more comment about what agreement I should see when. If I choose TDAP only, I get an OHEP agreement (only). Wasn't sure if that was supposed to happen?

label=#{${label}},
content=~{::checkboxInSetContent})}">
<th:block th:ref="checkboxInSetContent">
<div th:with="inputName='someone' + ${question}, label='sensitive-conviction-question-' + ${question}, helpText='sensitive-conviction-question-' + ${question} + '-helpText'">
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this line, but putting here. I noticed that the questions for violating parole and trading/trafficking SNAP are reversed from what the design has. Not sure if that's significant or by design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updating.

@bseeger bseeger self-requested a review February 15, 2024 20:01
Copy link
Contributor

@bseeger bseeger left a comment

Choose a reason for hiding this comment

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

Minor comments. Feel free to merge once addressed.

@@ -16,6 +16,6 @@ public class SetDefaultSensitiveConvictionAnswers implements Action {

@Override
public void run(Submission submission) {
Arrays.stream(SubmissionUtilities.SENSITIVE_CONVICTION_QUESTIONS).forEach(s -> submission.getInputData().putIfAbsent("noOne" + s + "[]", "true"));
Arrays.stream(SubmissionUtilities.SENSITIVE_CONVICTION_QUESTIONS).forEach(s -> submission.getInputData().putIfAbsent("someone" + s, "false"));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love this pattern.

@@ -394,10 +394,17 @@ void docUploadSkipTest() {
@Test
void sensitiveConvictionQuestionsArePreCheckedTest() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small thing:

sensitiveConvictionQuestionsArePrecheckedNoTest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, went with sensitiveConvictionQuestionsArePreCheckedToFalseTest

@analoo
Copy link
Collaborator

analoo commented Feb 15, 2024

PR looked good to me. Only general feedback is a question around web addresses: Do we always link the actual page or is that something we only do if the design requires it. For example there are a few places in rights and responsibilities where I can see a URL but I don't have the ability to select it:

Screenshot 2024-02-15 at 1 55 02 PM

@vrajmohan
Copy link
Contributor Author

vrajmohan commented Feb 15, 2024

@analoo , I missed a spot in formatting URLs. And thanks to that, I found another :-(

@vrajmohan vrajmohan merged commit 9f6ca82 into main Feb 15, 2024
4 checks passed
@vrajmohan vrajmohan deleted the complete-sign-and-submit-187003764 branch February 16, 2024 04:13
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