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-13555: Remove hidden attribute for skip to content #1716

Merged
merged 1 commit into from
Sep 25, 2023

Conversation

chriskim2311
Copy link
Contributor

@chriskim2311 chriskim2311 commented Sep 22, 2023

Description

Remove the hidden attribute when clicking the skip to content button

Testing done & Screenshots

Verified that the hidden attribute is not being added to the skip to content link. Tabbing back through the content shows the skip to content link again.

Old Behavior:

Screen.Recording.2023-09-25.at.9.57.52.AM.mov

New Behavior:

Screen.Recording.2023-09-25.at.9.56.02.AM.mov

QA Steps

  1. Go to any page on VA.gov, for example: https://www.va.gov/health-care/about-va-health-benefits/
  2. Start tabbing through the page immediately
  3. You will see the Skip to content link appear once you focus on it
  4. Hit enter to activate the link and jump down to the main content portion of the page
  5. Now tab backwards through the page (SHIFT + Tab) to try and get back to the Skip to Content link.

Acceptance criteria

  • a11y review
  • The Skip to content link should always be available, even after it has been activated.

@chriskim2311 chriskim2311 changed the title remove hidden attribute for skip to content VACMS-13555: Remove hidden attribute for skip to content Sep 25, 2023
@chriskim2311 chriskim2311 marked this pull request as ready for review September 25, 2023 16:33
@chriskim2311 chriskim2311 requested review from a team as code owners September 25, 2023 16:33
@chriskim2311
Copy link
Contributor Author

@laflannery This is the final PR for the skip to content link, let me know if you find any issues!

@randimays
Copy link
Contributor

We need to add more detail to PRs like this. A link to the ticket on any PR is required (unless there is no ticket). Screenshots or video would be helpful. QuickTime player is good for capturing video if you need to demonstrate changed behavior. And be sure to include the ACs of the story that would be relevant to the PR; reviewers need to know what to be testing for, also.

@chriskim2311
Copy link
Contributor Author

Thanks Randi, Let me update this PR to make it more detailed!

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

Tested multiple pages, Looks good! Appproved @chriskim2311

@chriskim2311 chriskim2311 merged commit 75616b4 into main Sep 25, 2023
24 checks passed
@chriskim2311 chriskim2311 deleted the 13555-skip-to-content-fix branch September 25, 2023 18:29
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