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

Add keyboard shortcut ("/" key) to focus search input #380

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

Conversation

navidadelpour
Copy link

Overview

This pull request adds a keyboard shortcut ("/" key) functionality in GraphQL Voyager, enabling users to focus on the search input. This enhancement aims to improve accessibility and empower users to search without relying on mouse interactions.

Peek 2024-01-05 21-20

This commit introduces a new feature that allows users to focus the search input by pressing the "/" key on the keyboard. Upon pressing "/", the search input will be automatically selected, improving accessibility and user experience within the GraphQL Voyager interface.
@navidadelpour
Copy link
Author

Hi @IvanGoncharov,

Can you please review this PR and let me know if there is any issue?

Copy link
Member

@IvanGoncharov IvanGoncharov left a comment

Choose a reason for hiding this comment

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

@navidadelpour Good idea 👍
Thank you for PR.

Since we heavily depend on MUI we should use their component to handle hotkeys.
I did a quick googling and found this:
https://github.com/mui/material-ui/pull/10725/files

Can please try the same approach in Voyager?

@navidadelpour
Copy link
Author

@IvanGoncharov thanks for reviewing.

I will definitely check that in the next couple of days and let you know when it is updated.

@navidadelpour
Copy link
Author

@IvanGoncharov Hi,

I Updated the PR and it's much more simpler. thanks for guidance. I didn't installed the react-event-listener because it's not compatible with react 18 and is archived on Github, I used an alternative package.

Also, as I was checking the PR#10725 you've sent and MUI code, I realized that this PR is not actually merged to the main branch right now. they are using @docsearch/react package and the way the package is implementing this feature is here.

Can you please review again?

@LunaticMuch
Copy link
Collaborator

@navidadelpour can you actually run tests and lint on your branch? PR tests for linting are failing

@navidadelpour
Copy link
Author

Hi @LunaticMuch

Thanks for reminding me. I should have run them before my push! Would you please check again?

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.

3 participants