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(quicksearch): opened pages don't scroll to top #8911

Merged
merged 3 commits into from
May 23, 2023

Conversation

LeoMcA
Copy link
Member

@LeoMcA LeoMcA commented May 22, 2023

Summary

https://mozilla-hub.atlassian.net/browse/MP-400

Problem

  • page doesn't scroll to top when using quicksearch
  • when using keyboard to open a result, key modifiers aren't recognised, so pages can't be opened in e.g. a new tab

Solution

  • use proper links
  • dispatch a MouseEvent when using the keyboard - unfortunately, browser support for passing modifiers is inconsistent, I presume this is some kind of clickjacking protection

How did you test this change?

  • clicked on quicksearch results with mouse, using various modifier keys (or none)
  • interacted with quicksearch results with keyboard, using various modifier keys (or none) across Firefox/Chromium/Epiphany (with inconsistent/consistent/no support for modifier keys respectively)

https://mozilla-hub.atlassian.net/browse/MP-400

do this by removing use of useNavigate, and using "proper links"

also attempt to pass modifier keys to link click when using the keyboard
browser support is inconsistent, but it works in some cases
@LeoMcA LeoMcA requested a review from fiji-flo May 22, 2023 16:14
Copy link
Contributor

@fiji-flo fiji-flo left a comment

Choose a reason for hiding this comment

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

Looks good.

@fiji-flo fiji-flo merged commit 964251b into main May 23, 2023
@fiji-flo fiji-flo deleted the mp-400-quicksearch-nav branch May 23, 2023 12:47
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.

2 participants