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

Fix the skip-to-content link reloading the results #3942

Merged
merged 2 commits into from
Mar 21, 2024

Conversation

obulat
Copy link
Contributor

@obulat obulat commented Mar 20, 2024

Fixes

Fixes #3940 by @sarayourfriend

Description

This PR converts the skip-to-content link to an a anchor link instead of using the VLink component because this component links to a hash #content URL, and VLink introduces unnecessary complexity for it.
To stop the media re-loading when clicking on skip-to-content, I added a check for hash in the search.ts middleware. We don't update anything in the middleware if the navigation was caused by a hash change.

Testing Instructions

Try searching for something, and then use Tab to focus the Skip-to-content button, click on the button to navigate to content. You should not see the media reload or the content links change number of results.

Checklist

  • My pull request has a descriptive title (not a vague title likeUpdate index.md).
  • My pull request targets the default branch of the repository (main) or a parent feature branch.
  • My commit messages follow best practices.
  • My code follows the established code style of the repository.
  • I added or updated tests for the changes I made (if applicable).
  • I added or updated documentation (if applicable).
  • I tried running the project locally and verified that there are no visible errors.
  • I ran the DAG documentation generator (if applicable).

Developer Certificate of Origin

Developer Certificate of Origin
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@github-actions github-actions bot added the 🧱 stack: frontend Related to the Nuxt frontend label Mar 20, 2024
@openverse-bot openverse-bot added 🟥 priority: critical Must be addressed ASAP 🛠 goal: fix Bug fix ♿️ aspect: a11y Concerns related to the project's accessibility 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work labels Mar 20, 2024
@obulat obulat removed the 🚦 status: awaiting triage Has not been triaged & therefore, not ready for work label Mar 20, 2024
@obulat obulat force-pushed the fix/skip-to-content-button branch from e1b613c to b062158 Compare March 20, 2024 14:16
@obulat obulat self-assigned this Mar 20, 2024
@obulat obulat marked this pull request as ready for review March 20, 2024 15:47
@obulat obulat requested a review from a team as a code owner March 20, 2024 15:47
Copy link
Member

@zackkrida zackkrida left a comment

Choose a reason for hiding this comment

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

Excellent! There is one thing that is broken but it is a one line fix, so I will pre-approve knowing you will add the fix so we can get this critical bug resolved and unblock deployments.

Copy link
Collaborator

@AetherUnbound AetherUnbound left a comment

Choose a reason for hiding this comment

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

This works as expected locally! I don't see the results reload when using the button.

@obulat obulat merged commit 3ed38fc into main Mar 21, 2024
39 checks passed
@obulat obulat deleted the fix/skip-to-content-button branch March 21, 2024 03:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
♿️ aspect: a11y Concerns related to the project's accessibility 🛠 goal: fix Bug fix 🟥 priority: critical Must be addressed ASAP 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Using "skip to content" button on search results page clears result counts (staging only)
4 participants