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

VACMS-15717: Change reusable Q&As header structure #1845

Merged
merged 8 commits into from
Jan 10, 2024

Conversation

chriskim2311
Copy link
Contributor

@chriskim2311 chriskim2311 commented Dec 28, 2023

Note: Delete the description statements, complete each step. None are optional, but can be justified as to why they cannot be completed as written. Provide known gaps to testing that may raise the risk of merging to production.

Summary

For this ticket we addressed header tags for accordions based on if we have a header title or not. There were multiple implementations of these reusable Q&As so we created tugboat content for each.

 FAQs
 Resources & Support
 Benefit Detail Pages
 CLPs

Related issue(s)

department-of-veterans-affairs/va.gov-cms#15717

Testing done

Tugboat Env: main-dyel3cg3kniaoc6nrizjhqazkfgueref.demo.cms.va.gov/

CLP: https://web-dyel3cg3kniaoc6nrizjhqazkfgueref.demo.cms.va.gov/initiatives/this-is-my-clp
FAQs: https://web-dyel3cg3kniaoc6nrizjhqazkfgueref.demo.cms.va.gov/resources/test-faq-page
Resources & Support: https://web-dyel3cg3kniaoc6nrizjhqazkfgueref.demo.cms.va.gov/resources/r-s-detail-page
Benefit Detail Pages: https://web-dyel3cg3kniaoc6nrizjhqazkfgueref.demo.cms.va.gov/benefits-detail

Screenshots

What areas of the site does it impact?

(Describe what parts of the site are impacted if code touched other areas)

Acceptance criteria

  • Test Reusable Q&As across multiple content types.
  • A11y review

Quality Assurance & Testing

  • I fixed|updated|added unit tests and integration tests for each feature (if applicable).
  • No sensitive information (i.e. PII/credentials/internal URLs/etc.) is captured in logging, hardcoded, or specs
  • Linting warnings have been addressed
  • Documentation has been updated (link to documentation *if necessary)
  • Screenshot of the developed feature is added
  • Accessibility testing has been performed

@chriskim2311 chriskim2311 changed the title change single qa to have h3 headers VACMS-15717: Change single qa to have h3 headers Dec 28, 2023
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers December 28, 2023 00:33 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers January 5, 2024 23:13 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers January 6, 2024 01:37 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers January 8, 2024 19:55 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers January 8, 2024 21:44 Inactive
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/15717-resuable-qa-headers January 8, 2024 23:09 Inactive
@chriskim2311 chriskim2311 changed the title VACMS-15717: Change single qa to have h3 headers VACMS-15717: Change reusable Q&As header structure Jan 9, 2024
@chriskim2311 chriskim2311 marked this pull request as ready for review January 9, 2024 16:31
@chriskim2311 chriskim2311 requested review from a team as code owners January 9, 2024 16:31
Copy link
Contributor

@randimays randimays left a comment

Choose a reason for hiding this comment

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

LGTM!

@laflannery
Copy link

These look good, however, per discussion with with @chriskim2311 I think we may need to rethink/rework the CLP headers when we looked at this other ticket

Copy link
Contributor

@maxx1128 maxx1128 left a comment

Choose a reason for hiding this comment

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

This may be because I'm less familiar with this content, but it looks like some h2 headers appear near the bottom of most of the pages here. Is that intended or is the code not keeping them at h3 when it should be?

Screen Shot 2024-01-09 at 1 02 48 PM Screen Shot 2024-01-09 at 1 03 49 PM

@laflannery
Copy link

@maxx1128 I don't understand the question? However I can tell you that this is fake content - does that help answer your question?

@maxx1128
Copy link
Contributor

maxx1128 commented Jan 9, 2024

@laflannery My question is whether the circled headers should be h2 or h3 headers. The fact that it's fake data makes it harder for me to tell if them being h2s was purposeful or accidental.

@laflannery
Copy link

They should be H2s - I reviewed these pages previously and the header structure is now all correct, pending the comment I left before

@chriskim2311 chriskim2311 merged commit 31943d8 into main Jan 10, 2024
25 checks passed
@chriskim2311 chriskim2311 deleted the 15717-resuable-qa-headers branch January 10, 2024 19:00
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.

6 participants