Nj 243 - Add headers to review screen

[jachan]

Link to pivotal/JIRA issue

https://github.com/newjersey/affordability-pm/issues/243

Is PM acceptance required? (delete one)

• Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

• Add "Deductions and Credits" and "Your 2024 Taxes" Headers to NJ review screen to match content doc:https://docs.google.com/document/d/1I3IAqk1xYehwF3iJEw0hBUXLmD3db4rM_Ivm7HBfnbo/edit?tab=t.0#heading=h.buaa78ggxvdj
• Update headers so that the hierarchy is correctly maintained
  • Note that the abstract header used has h2 headings, so the titles of each deduction/credit need to be set to h3.

How to test?

• Navigate to nj_review, validate that the headings appear correctly in english and spanish

Screenshots (for visual changes)

• Before

• After

[github-actions]

Heroku app: https://gyr-review-app-5563-299e99447ffd.herokuapp.com/
View logs: heroku logs --app gyr-review-app-5563 (optionally add --tail)

[mmazanec22]

This structure makes more sense overall! One [pebble] I noticed: income forms collected is not an h3, which seems to be inconsistent with how we're structuring other boxes

[mmazanec22]

[pebble ] The purpose of the test before was to make sure every white-group section box began with an h2, for consistent structure. Hence comparing the number of h2s to the number of white group boxes.

Now that we're beginning boxes with h3s instead, I think it makes sense to check that structure instead of just counting elements. Maybe checking that each white group has at least one h3? And that it comes before any other content?

[jenny-heath]

^ agree that just checking the totals (white groups = h3s) makes more sense. this looks like it's checking that each section has an h3 but since it just adds them up in the end it's actually not a very specific test

[jachan]

The issue with the previous test is that not every white group on the page starts with a single h3 heading. Examples:

• Income review white-group does not have any heading
  

  vita-min/app/views/state_file/questions/shared/_review_header.html.erb

  Line 6 in c75ad16

  <div id="income-info" class="white-group">

• Refund or owed amount white-group uses an h2 heading, and can't be switched to h3 or it will fail axe validation
  

  vita-min/app/views/state_file/questions/shared/_abstract_review_header.html.erb

  Line 9 in c75ad16

  <div class="white-group">

• Household info has multiple h3 headings, one for self, one for spouse, and one for each dependents:
  

  vita-min/app/views/state_file/questions/shared/_abstract_review_header.html.erb

  Line 40 in c75ad16

  <% current_intake.dependents.each_with_index do | dependent, index | %>

The previous test (white-groups=h2s) was not testing what we wanted to test , since abstract_header had been using h3 headings in its white-group sections and we were previously checking against h2 headings.

https://github.com/codeforamerica/vita-min/blob/c75ad162f29742abd773e98eba6e566608530ea3/app/views/state_file/questions/shared/_abstract_review_header.html.erb

I started trying to modify the math to make it work, but with all the cases above, it seemed much more readable to just assert what I expected for each section. Happy to explore other options if folks have suggestions, but just pulling the # of white groups and trying to modify that to match the number of h3s required a lot of arithmetic that seemed really opaque.

[jachan]

Just read Melanie's comment more closely and found the suggestion to check for the presence of a header before any other content in the white-group, which would make a lot more sense here rather than just asserting that every white-group has an h3. Exploring that solution now!

[mmazanec22]

We should use consistent heading structure, and the visual appearance should reflect the markup. It doesn't make sense to start some white groups with h2s and others with h3s-- all of the content should be broken into sections if any of it is. The design might need to be tweaked if this isn't currently possible

[jenny-heath]

typo?

Suggested change

	<h3 class="text--body text--bold spacing-below-5"><%=t(".college_dependents") %></h2>
	<h3 class="text--body text--bold spacing-below-5"><%=t(".college_dependents") %></h3>

[jachan]

Oh my gosh! Thanks for catching, I'm so sorry - I forgot to change all these closing tags. Will put out a revision.

[jenny-heath]

Suggested change

	<h3 class="text--body text--bold spacing-below-5"><%=t(".missing_health_insurance") %></h2>
	<h3 class="text--body text--bold spacing-below-5"><%=t(".missing_health_insurance") %></h3>

[jenny-heath]

Suggested change

	<h3 class="text--body text--bold spacing-below-5"><%=t(".year_of_death") %></h2>
	<h3 class="text--body text--bold spacing-below-5"><%=t(".year_of_death") %></h3>

[jenny-heath]

Suggested change

	<h3 class="text--body text--bold spacing-below-5"><%=t(".county_municipality", filing_year: current_tax_year) %></h2>
	<h3 class="text--body text--bold spacing-below-5"><%=t(".county_municipality", filing_year: current_tax_year) %></h3>

[jenny-heath]

i.....didn't realize how many of these there are. you get the point 🙈

[mluedke2]

[a mound of pebbles] - looks like the h2 -> h3 change was only on the opening tag and not on the closing one. And that seems to apply to all the changes in this PR. it might still render correctly in browsers and test drivers which would hide the issue, but we should fix it

[jenny-heath]

gotta fix the closing tags! (also would love to see melanie's testing suggestion implemented but that's not blocking)