-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Improved search feature for API docs #5229
Conversation
@@ -78,6 +78,7 @@ class Crystal::Doc::Generator | |||
|
|||
main_index = Main.new(raw_body, Type.new(self, @program), repository_name) | |||
File.write File.join(@output_dir, "index.json"), main_index | |||
File.write File.join(@output_dir, "index.jsonp"), main_index.to_jsonp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think search-index.js
(from Rust doc) is better than index.jsonp
. The extension jsonp
is irregular.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, why JSONP? Still, there might a problem with loading json
files over file:///
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
search-index.js
seems like a good idea 👍
@Sija Yes, browsers can't simply load arbitrary files over We could also encourage people to load the docs through a local HTTP server like serve (or at best include one directly into the docs generator) but this is always one step more complicated and the workaround with JSONP is super easy. |
34a51af
to
4a2845e
Compare
Why isn't it Would be fantastic if the search handled typos by giving some weight to keys that are close physically to the one actually pressed, but thats really way too much to ask especially with the number of keyboard layouts we have. |
Bug reports:
There should also probably be a (x) button to clear the search box. The small side bar is also too small (for search) on vertical monitors. I'm not sure exactly how to solve that. Perhaps a popout search, or expand the sidebar on search below a window size. Also, it's not a true fuzzy search, i'd expect |
Thanks for the testing.
|
The solution is to define a peek button p which does the same as enter but focusses the search bar back again. You could even add ctrl-p so you don't need to press the down arrow |
@RX14 I'm not sure, I can follow. |
23cb560
to
7d23a89
Compare
@straight-shoota the problem is that it's unexpected. When i'm at the docs my muscle memory is to do The rule should be that after I press enter I should be exactly the same node of the search state machine as I am when the page loads. And the starting node shouldn't depend on anything, a refresh should be a completely blank slate. For people who want your behaviour of keeping the search results to browse through them, they should have a "peek" shortcut (ctrl-p) to peek the results of a search result, but they're still at the search list so they can find another result if it's incorrect. I'll probably never use peek. 99% of the time i've arrived in the right file, or at the least i'll need to correct my search term not just move to the second result. In fact i'll rarely be moving to the second result, largely my intended result will be rank 1, or i'll be refining my search term. |
Another thing which would greatly help for muscle memory would be making it so that pressing |
If I understand you correctly, with "Highlight the results of the search box" you mean "select the text in the query input field"? |
@straight-shoota yep. |
@straight-shoota have you had any more time to work on this? |
@RX14 I've implemented that the results list is not automatically reloaded on a new page and when you focus the search field, the content gets selected. I haven't done the peek feature yet. Preview link has been updated. |
@straight-shoota bug: typing |
Woah, yep. That event shouldn't trigger when the input is selected. |
I've fixed this and added a modal window listing shortkeys (can be opened with |
Awesome improvement to documentation 🎉 Would be possible to hide sidebar on small screens and add a hamburger button? |
@faustinoaq Yes, this is the next step after #4755 |
I think that the input box should be explicitly cleared and unfocussed at |
@RX14 This is actually done on purpose. Each search query is stored in session storage and the last value is retrieved on page load. This is intended to help refine a query so you don't have to type again. But I see that the combination of both can be confusing, and maybe we could remove the auto-filling but keep the focus. |
@straight-shoota autofilling is actually OK, since I tend to use the keyboard much more than mouse to scroll webpages. |
Being mostly a mouse user I didn't put that into consideration. |
@straight-shoota have you updated the preview site? I still get the focus on the search box. |
Oh, sry. |
Fixed it, preview is now correct. |
When a search result is opened, the new page does not load the results list and the query field is not focused (`leaveSearchScope`) To refine or start a new search, when the query field is focused (`q` or `/` shortcut) it's content is selected.
`scrollIntoView({behaviour: smooth, block: center})` is only an experimental DOM feature and not supported by many browsers.
69ff27c
to
9b83d32
Compare
🎉 🎉 🎉 I've been waiting for this forever. Can't wait for the first official crystal release with these docs on crystal-lang.org. |
@RX14 The current docs could already be upgraded to a version built by a compiler including this PR. |
In fact, I would really appreciate it if it could be updated soon. That would allow us to catch and fix errors (specifically improvements for the result ranking) which will only emerge if it is broadly used. This way we could get fixes before the next release. |
@straight-shoota I'd be okay with this, but the rest of the core team should weigh in. |
Using better docs for the current version sounds good to me. |
* Implement advanced search for API docs This greatly improves client-side search on generated HTML pages of the API docs. It uses the program description available as JSON data and implements a custom search algorithm. The search results include types, constants, methods, macros matching the query string in one or more of the fields name, namespace, description, arguments or type. A custom ranking algorithm sorts results based on matching quality and relevance. Search features: * prefix filter: `#` matches instance methods, `.` matches class methods and macros, `:` matches namespace * search is generally case-insensitive but results with case-equality are ranked higher. * search results list can be navigated by key (when in focus): * `i`, `c`, `ArrowUp`: move upwards * `j`, `h`, `ArrowDown`: move downwards * `Enter`: load result * `Escape`: cancel search * arrows, `Enter` and `Escape` also work when the search-input is in focus * support for URL fragment-based search query: `/#q=mysearch` will initialize a search at page load * page-wide shortkeys to start search: `s` or `/` * Add jsonp support to allow search when API docs are loaded from local file system * Add jsonp support to allow search when API docs are loaded from local file system * Navigator: Fix keybindings, don't jump to result while searching * Improve result ranking based on in-string match position * Don't keep results list when clicking on result When a search result is opened, the new page does not load the results list and the query field is not focused (`leaveSearchScope`) To refine or start a new search, when the query field is focused (`q` or `/` shortcut) it's content is selected. * Fix: don't trigger global shortkeys when input has focus * Add usage modal * remove autofocus of search input * Delay navigator event after search has been performed * Keep current selected result centered on screen * Remove scroll position persistance (interferes with results list) * Delay execution of search until index is loaded * Add custom implemention for centered scrolling position `scrollIntoView({behaviour: smooth, block: center})` is only an experimental DOM feature and not supported by many browsers.
This PR greatly improves client-side search on generated HTML pages of the API docs.
It uses the program description available as JSON data from #4746 and implements a custom search algorithm in JavaScript.
The search results include types, constants, methods, macros matching the query string in one or more of the fields name, namespace, description, arguments or type.
A custom ranking algorithm sorts results based on matching quality and relevance.
The search-algorithm is actually quite simple, it uses no search index or advanced information retrieval functions. It simply traverses through all the types and tries to find matching features. This is really just fast enough.
I considered using something like lunr.js but this would have been complicated to setup to create a domain-specific search experience. With the custom algorithm it is very easy to fine tune matching and ranking behaviour to allow developers to find what they need as fast as possible.
This approach has some drawbacks with regards to fulltext search capabilities (especially concerning docstrings), but this can certainly be improved upon.
Search features:
#
matches instance methods,.
matches class methods and macros,:
/::
matches namespacei
,c
,ArrowUp
: move upwardsj
,h
,ArrowDown
: move downwardsEnter
: load resultEscape
: cancel searchEnter
andEscape
also work when the search-input is in focus/#q=mysearch
will initialize a search at page loads
or/
Preview
Online Preview
This is intended to be alongside #4755 which makes it look even better (Online Preview) though it can be merged separately.
Closes #1792