Open search config + search page #217
Conversation
Now we need to adapt the services to work correctly with the search and suggestion URLs.
… into account, adjusted the javascript to work with an initial value in the search form.
…er go back to the home page.
|
The PR is now ready to be reviewed and merged if appropriate. I left the suggestion endpoint unfinished, it will make more sense to implement it once we have a better way to do the search. |
| </Url> | ||
| <Url type="application/x-suggestions+json" template="https://cljdoc.org/suggest"> | ||
| <Param name="q" value="{searchTerms}"/> | ||
| </Url> |
There was a problem hiding this comment.
would it perhaps make sense to comment that out for now to avoid unnecessary requests?
There was a problem hiding this comment.
I chose to have the suggestion part in the descriptor from the beginning because once the users click on the ➕ icon to adopt the custom search engine, the descriptor used by the browser may be a copy and may not automatically update with the website later.
As for the server load concern, the user seems to need to activate the suggestion options in the browser's preference tab in order to use it. We can expect only a small minority to choose cljdoc instead of google as their primary search engine, so that should normally not be a problem.
Let me know if you still prefer that I remove it.
There was a problem hiding this comment.
Thanks for the context, makes sense 👍
|
|
||
| ;; Open Search | ||
| [:link {:rel "search" :type "application/opensearchdescription+xml" | ||
| :href "/opensearch.xml" :title "Cljdoc"}] |
There was a problem hiding this comment.
cljdoc (all lower case) would be more "on brand" 😄
| ([] (search-form "")) | ||
| ([search-terms] | ||
| [:div.w-90.mb4 | ||
| [:div#cljdoc-search {:initial-value search-terms}]])) |
There was a problem hiding this comment.
Maybe data-initial-value is more appropriate here, but again, not very important.
There was a problem hiding this comment.
| ;; See https://developer.mozilla.org/en-US/docs/Web/OpenSearch for more information. | ||
| (defn suggest-api | ||
| [context] | ||
| (assoc context :response {:status 200 |
There was a problem hiding this comment.
Maybe sending a 404 here with no content would be more appropriate until this is implemented?
There was a problem hiding this comment.
Good point. I found the code 501 "not implemented" that fits better, will use it instead.
|
Thanks for the adjustments!🙌 |
2 participants
References the issue #186
This PR is not ready to be merged yet. Please provide feedback on the code change.
The search page still need a link for the user to go back to the home page.