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

feat(v3): allow overriding default toggle key #1196

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

shortcuts
Copy link
Member

@shortcuts shortcuts commented Nov 29, 2021

Summary

This PR adds a new option to the DocSearch and DocSearchModal components to override the default toggleKey (k) shortcut.

Motivations

While cmd+k usage slowly spread, we can understand that users would like to use something else (e.g. cmd+f) without having to reimplement the whole component.

@netlify
Copy link

netlify bot commented Nov 29, 2021

✔️ Deploy Preview for docsearch ready!

🔨 Explore the source changes: 23475f9

🔍 Inspect the deploy log: https://app.netlify.com/sites/docsearch/deploys/61a4ebf4e9ff330008631128

😎 Browse the preview: https://deploy-preview-1196--docsearch.netlify.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Nov 29, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 23475f9:

Sandbox Source
Vanilla Configuration

Copy link
Member

@francoischalifour francoischalifour left a comment

Choose a reason for hiding this comment

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

This is the kind of feature that we want to be opinionated about, and not create disparities in the DocSearch ecosystem. Users can already use le lower-level components to customize the keyboard shortcuts.

@shortcuts
Copy link
Member Author

This is the kind of features that we want to be opinionated about, and not create disparities in the DocSearch ecosystem.

It's clearly the reason why I wasn't sure about adding it to the options, but it's requested quite frequently so it's better to have a public discussion.

On the other hand, I still think it's good to provide a bit of customization without having to re-implement the component.

@francoischalifour
Copy link
Member

We don't necessarily want to please everybody in all situations. The K pattern was fully studied and introduced with a clear plan in mind: democratize a mental pattern for developers when using docs. We don't want to send blurry signals and have developers wonder if hitting K would start searching or not. Our take is that it should. We shouldn't make it an option just because we can – options are the main path to confusion.

DX is also about constraints. If we don't recommend customizing the keyboard shortcut, we shouldn't make it easy to do so.

@bodinsamuel
Copy link
Contributor

I did not had strong opinions on this, but I'll share what's top on my mind.

  • If this has been requested, can we gather why it has been requested?
  • k might already been assigned, if you have Docsearch on top of VSCode for example.
  • k might not fit all lang (I don't know about asian languages but I guess there might be an issue here)
  • Docsearch is a tool not a standard imo, having a good DX by default should not disallow customisation, and I'm sure it will be okay for 99% of the implems.
  • "Command palette" pattern is more and more prevalent in SaaS, it might be a blocker to integrate Docsearch, but in that case they might want to remove have their own search anyway.

My 2 cents on this.

@Shipow
Copy link
Contributor

Shipow commented Nov 30, 2021

Agree with Francois, low level customization should be enough in that case.

@shortcuts
Copy link
Member Author

shortcuts commented Nov 30, 2021

options are the main path to confusion.

I agree and this is why I wanted to have this open to a possible debate.

DX is also about constraints.

This is exactly why I first doubted when opening this PR, but we have developers with different background using our tool. While providing the best DX (by default in that case) is obviously our concern, we should also ease their journey when they want to move out of our constraints.

low level customization should be enough in that case.

I'd also add that almost half of our users are using website generators and only see DocSearch components through their website config file, providing an option at this level is also a great DX improvement.

@bodinsamuel
Copy link
Contributor

If we all have different opinions we need facts:

why it has been requested?

@bodinsamuel bodinsamuel removed their request for review January 4, 2022 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants