-
Notifications
You must be signed in to change notification settings - Fork 13
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
Nj 211 separate health #5307
Nj 211 separate health #5307
Conversation
Heroku app: https://gyr-review-app-5307-06bac2b4a196.herokuapp.com/ |
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.
Looks good! Nice catch on using tax year rather than current date year 😅
My main note here is that depending on what gets merged first, this or the review page, one of those tickets should update the (newly created) review section for this question to have the edit button go back to this new controller.
config/locales/en.yml
Outdated
@@ -2119,6 +2119,7 @@ en: | |||
nc_tax_withheld: North Carolina tax withheld | |||
nc_taxable_income: North Carolina taxable income | |||
nc_use_tax: North Carolina use tax | |||
nj_minimal_essential_health_coverage_html: "<p>Most people who have health insurance have minimum essential coverage. “Minimal essential coverage” just refers to how much your plan covers. If you have any of the following, you most likely have it:</p>\n<ul>\n <li>A plan through your employer</li>\n <li>A plan bought through <a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://nj.gov/getcoverednj/\">GetCoveredNJ</a></li>\n <li>Medicare</li> \n <li>Medicaid (NJ FamilyCare)</li>\n</ul>\n<p><a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://nj.gov/treasury/njhealthinsurancemandate/getinfo.shtml\">Here is the full list of insurance options that have minimum essential coverage.</a><p> \n<p>If you only had standalone vision and dental plans, workers' compensation coverage, and/or coverage limited to a specified disease or illness, you didn't have minimum essential coverage. In New Jersey, as of 2018, you need minimum essential coverage, or you will likely pay additional taxes.</p>\n" |
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.
[dust] do we have a guideline on when to use \n
vs <br>
in these translation strings?
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.
[dust] those \n
look to me like they were added by the ide when pasting the html block inside quotes, and don't actually have any impact on this string once it is parsed.
in terms of best practice for translated html strings, I think this example is much more readable and avoids the ide adding in \n
end | ||
end | ||
|
||
context "when intake has depedents" do |
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.
[dust] typo dependents
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.
looks good and no blockers from me!
config/locales/en.yml
Outdated
@@ -2119,6 +2119,7 @@ en: | |||
nc_tax_withheld: North Carolina tax withheld | |||
nc_taxable_income: North Carolina taxable income | |||
nc_use_tax: North Carolina use tax | |||
nj_minimal_essential_health_coverage_html: "<p>Most people who have health insurance have minimum essential coverage. “Minimal essential coverage” just refers to how much your plan covers. If you have any of the following, you most likely have it:</p>\n<ul>\n <li>A plan through your employer</li>\n <li>A plan bought through <a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://nj.gov/getcoverednj/\">GetCoveredNJ</a></li>\n <li>Medicare</li> \n <li>Medicaid (NJ FamilyCare)</li>\n</ul>\n<p><a target=\"_blank\" rel=\"noopener noreferrer\" href=\"https://nj.gov/treasury/njhealthinsurancemandate/getinfo.shtml\">Here is the full list of insurance options that have minimum essential coverage.</a><p> \n<p>If you only had standalone vision and dental plans, workers' compensation coverage, and/or coverage limited to a specified disease or illness, you didn't have minimum essential coverage. In New Jersey, as of 2018, you need minimum essential coverage, or you will likely pay additional taxes.</p>\n" |
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.
[dust] those \n
look to me like they were added by the ide when pasting the html block inside quotes, and don't actually have any impact on this string once it is parsed.
in terms of best practice for translated html strings, I think this example is much more readable and avoids the ide adding in \n
470c3a9
to
094926b
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Link to pivotal/JIRA issue
https://github.com/newjersey/affordability-pm/issues/211
Is PM acceptance required? (delete one)
What was done?
How to test?
Use many deps claimed as dep profile. Health insurance eligibility question should not show, but dependent health insurance should show after dependents in college question.
Screenshots (for visual changes)