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-28709 in this section a11y iOS fix #2409

Merged
merged 5 commits into from
Jan 9, 2025

Conversation

chriskim2311
Copy link
Contributor

@chriskim2311 chriskim2311 commented Jan 7, 2025

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

In this PR we add markup to the div containing the accordion dropdown to allow the cursor in iOS to stay once the drawer is open.

Related issue(s)

department-of-veterans-affairs/va.gov-team#28709

Testing done

Hard to test this in iOS. But checked that the new markup is showing locally.

Screenshots

Health_And_Disability_Benefits_For_Family_And_Caregivers___Veterans_Affairs

What areas of the site does it impact?

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

Acceptance criteria

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

Error Handling

  • Browser console contains no warnings or errors.
  • Events are being sent to the appropriate logging solution
  • Feature/bug has a monitor built into Datadog or Grafana (if applicable)

@chriskim2311 chriskim2311 changed the title VACMS-28709 in this section a11y ios fix VACMS-28709 in this section a11y iOS fix Jan 7, 2025
@va-vfs-bot va-vfs-bot temporarily deployed to master/main/28709-in-this-section-a11y January 7, 2025 20:20 Inactive
@chriskim2311 chriskim2311 marked this pull request as ready for review January 8, 2025 17:09
@chriskim2311 chriskim2311 requested a review from a team as a code owner January 8, 2025 17:09
@laflannery
Copy link

@chriskim2311 Shamia and I took a look and we have a few comments:

  • Is it possible to have the ESC key available to close the modal/dialog? This is extremely helpful for SR users when dialogs are involved so they don't have to find the Close button.
  • We also noticed that the aria-labelledby attribute was missing on the dialog:
    Screenshot 2025-01-08 at 3 40 21 PM
  • And for the Nav itself, can you wrap this in a semantic <nav> tag with an `aria-label="Secondary" Like this has on desktop?
    Screenshot 2025-01-08 at 3 46 54 PM

We also noticed a difference in the styles regarding the active page and subnav, screenshots below:
Screenshot 2025-01-08 at 3 47 49 PM

@chriskim2311
Copy link
Contributor Author

@laflannery Added the changes from your comments ready for you to re-review thanks!

Copy link

@laflannery laflannery left a comment

Choose a reason for hiding this comment

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

Looks great, approved! Shamia and I both want to test this on real mobile though once it's merged. Will you ping one or both of us once it's in staging?

@chriskim2311 chriskim2311 merged commit f4568b5 into main Jan 9, 2025
24 checks passed
@chriskim2311 chriskim2311 deleted the 28709-in-this-section-a11y branch January 9, 2025 21:33
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.

4 participants