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.

Sign up for GitHub

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

Jump to bottom

Added keyboard shortcut for search box #2027

Merged
merged 11 commits into from
Feb 12, 2023
Merged

Added keyboard shortcut for search box #2027

merged 11 commits into from
Feb 12, 2023

Conversation

Hetarth02
Copy link
Contributor

@Hetarth02 Hetarth02 commented Jan 29, 2023 •
edited by mortenpi
Loading

Also, added .vscode to .gitignore
Link to keyboard shortcut is here.

To focus into search bar press ctrl + / or cmd + / keys.
To focus out of search bar press Esc key.

Edit by @mortenpi: close #1536.

@Hetarth02
- added keyboard shortcut for search box
b30b2e0 
- Added .vscode to .gitignore
@Hetarth02 Hetarth02 changed the title Added keyboard shortcut for search box as requested in this [Issue](https://github.com/JuliaDocs/Documenter.jl/issues/1536) Added keyboard shortcut for search box as requested in this Issue(https://github.com/JuliaDocs/Documenter.jl/issues/1536) Jan 29, 2023
@Hetarth02 Hetarth02 changed the title Added keyboard shortcut for search box as requested in this Issue(https://github.com/JuliaDocs/Documenter.jl/issues/1536) Added keyboard shortcut for search box Jan 29, 2023
@mortenpi mortenpi added Type: Enhancement Format: HTML Related to the default HTML output labels Jan 29, 2023
@Hetarth02
Copy link
Contributor Author

How can I get that one job to be successful?

Is this PR accepted and be merged?

mortenpi
mortenpi reviewed Jan 31, 2023
View reviewed changes
Copy link
Member

@mortenpi mortenpi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks great, thanks!

  • I was wondering if we could have a small gray text in the search box, indicating the you can use Ctrl + / for the search? I think it would greatly help with the discoverability of this feature. Like in the Tailwind docs:

    image

    I guess it should be doable with some simple CSS maybe? But happy to merge as is too.

  • Could you add a brief note into CHANGELOG.md, with a reference to this this PR and the related issue?

.gitignore Outdated Show resolved Hide resolved
@mortenpi
Copy link
Member

mortenpi commented Feb 3, 2023

Ah, yeah, I forgot we show that text. That would be perfect. Probably just Search docs (ctrl + /) is sufficient.

@Hetarth02
Copy link
Contributor Author

Alright, will do the changes and make the commit.

@Hetarth02
- Removed Jquery dependency
ec10ac0 
- Changed placeholder to indicate the shortcut use
- Added the change in `CHANGELOG.md`
- Made the suggested edits in `.gitignore`
@Hetarth02
- Removed Jquery dependency
7c101ea 
- Changed placeholder to indicate the shortcut use
- Added the change in `CHANGELOG.md`
- Made the suggested edits in `.gitignore`
@Hetarth02
Merge branch 'master' of https://github.com/Hetarth02/Documenter.jl
617d7ac
@Hetarth02
Copy link
Contributor Author

@mortenpi I have made the suggested changes and some improvements to the method too.

  • The new code doesn't require JQuery to work, it is pure Javascript.

@fredrikekre
Copy link
Member

Why was Ctrl+/ chosen for this?

@Hetarth02
Copy link
Contributor Author

Why was Ctrl+/ chosen for this?

I didn't chose any single Alphanumeric character to avoid triggering the script while still being in the input.(For example, letter s to focus in and e to focus out. Now we wouldn't want the user to focus in and out while typing the term.)

For this reason, I assumed that a combination of keys would be ideal rather than any single character. Hope it clears up.

@mortenpi
Copy link
Member

mortenpi commented Feb 5, 2023

Why was Ctrl+/ chosen for this?

Fair question. Looking through a few sites, it doesn't look like there is massive consensus about this. GitHub uses just /, docs.rs uses just S. Vue uses Ctrl+K, but that actually apparently clashes with a Chrome shortcut. While I didn't immediately see any prior art for Ctrl+/, it seems like a reasonable choice.

mortenpi
mortenpi reviewed Feb 5, 2023
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
src/html/HTMLWriter.jl Outdated Show resolved Hide resolved
@fredrikekre
Copy link
Member

On many keyboards / requires shift though, so would be a three button combination.

@mortenpi
Copy link
Member

mortenpi commented Feb 5, 2023

We could maybe follow GitHub's lead and use just /?

mortenpi
mortenpi reviewed Feb 6, 2023
View reviewed changes
src/html/HTMLWriter.jl Outdated Show resolved Hide resolved
@Hetarth02
Copy link
Contributor Author

Hetarth02 commented Feb 6, 2023 •
edited
Loading

@mortenpi @fredrikekre Is there a final combination in mind?

P.S.: It can be changed into whichever combination(s) we like.

mortenpi
mortenpi reviewed Feb 12, 2023
View reviewed changes
.gitignore Outdated Show resolved Hide resolved
fredrikekre
fredrikekre approved these changes Feb 12, 2023
View reviewed changes
Copy link
Member

@fredrikekre fredrikekre left a comment

Choose a reason for hiding this comment

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

Lets try with Ctrl + /. If people prefer something else I think it would be perfectly fine to change this without requiring a breaking Documenter version.

src/html/HTMLWriter.jl Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
fredrikekre
fredrikekre reviewed Feb 12, 2023
View reviewed changes
.gitignore
@@ -1,3 +1,5 @@
.vscode
Copy link
Member

Choose a reason for hiding this comment

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

I don't mind that this is added, but note that you can use a global gitignore file for things that are personal (i.e. editor/workflow specific things). See e.g. https://docs.github.com/en/get-started/getting-started-with-git/ignoring-files#configuring-ignored-files-for-all-repositories-on-your-computer.

@fredrikekre fredrikekre merged commit 735eee7 into JuliaDocs:master Feb 12, 2023
@fredrikekre
Copy link
Member

After merging this I have noticed that Ctrl+K seems to be very common. Perhaps Documenter should update to that?

@mortenpi
Copy link
Member

I would be fine with that. I have also seen that used on some sites.

@mortenpi mortenpi mentioned this pull request Feb 28, 2023
Closed
@mortenpi
Copy link
Member

Just one more data point: apparently MultiDocumenter sites have a shortcut (to the MultiDocumenter global search) and use / (JuliaComputing/MultiDocumenter.jl#45).

@mortenpi mortenpi mentioned this pull request Apr 10, 2023
Open
@Hetarth02 Hetarth02 mentioned this pull request Aug 17, 2023
Merged
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredrikekre fredrikekre fredrikekre approved these changes

@mortenpi mortenpi mortenpi left review comments

Assignees
No one assigned
Labels
Format: HTML Related to the default HTML output Type: Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[request] search bar keyboard shortcut
3 participants
@Hetarth02 @mortenpi @fredrikekre