Skip to content

fix(plugin-docsearch): open on ctrl+k when page first loads #478

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

xav-ie
Copy link

@xav-ie xav-ie commented May 23, 2025

The { once: true } prevented the listener from properly attaching
and listening for a whole ctrl+k event. It would exit early, causing
the page to not function properly on first load.

useEventListener automatically removes itself on unmount, so I am not sure the purpose of this line. I am hopeful this is not intentional and simple mistake. By approving and merging this PR, you will greatly improve my quality of docs searching. Otherwise, I will need custom scripts :/.

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Provide a description in this PR that addresses what the PR is solving. If this PR is going to solve an existing issue, please reference the issue (e.g. close #123).

What is the purpose of this pull request?

  • Bug fix
  • New feature
  • Other

Description

The search feature now opens with ctrl+k consistently.

Screenshots

Before

before.mp4

After

after.mp4

The before branch adds the plugin inside the e2e thing. I am not sure if that is wanted. It requires some extra setup to test right now. I left that part out of this PR.

I am hopeful that once this is merged in that ctrl+k will work on sites like https://www.nushell.sh! :D

@xav-ie xav-ie marked this pull request as ready for review May 23, 2025 01:42
The `{ once: true }` prevented the listener from properly attaching
and listening for a whole ctrl+k event. It would exit early, causing
the page to not function properly on first load.
@xav-ie xav-ie force-pushed the fix-plugin-docsearch branch from 024eab2 to b908563 Compare May 23, 2025 01:50
event.preventDefault()
callback()
},
{ once: true },
Copy link
Author

Choose a reason for hiding this comment

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

I only removed this line. The rest is auto-formatting.

@Mister-Hope
Copy link
Member

Mister-Hope commented May 23, 2025

See algolia/docsearch#1363.

I will leave this for now, as the once: true does fix some issues.

Proper fix may need to be applied upstream

@xav-ie
Copy link
Author

xav-ie commented May 23, 2025

Thank you, Mister-Hope, for pointing me in correct direction. It now makes sense why code has to be this way for now.

I was able to find your old PR there and make it return an unmounting function. You are free to close this or keep it open and I will just make it use the unmounting function once available.

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