-
Notifications
You must be signed in to change notification settings - Fork 45
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
Incorporate Algolia-based search #642
Conversation
I'm down with that! We can always iterate, same with the other current annoyances. An MVP with search is already a vast improvement over what we have. |
@@ -0,0 +1,30 @@ | |||
const React = require("react") | |||
|
|||
exports.onRenderBody = ( |
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.
👍
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 don't know how to make this plugin work with ES module syntax, but it isn't something that bothers me. If it bothers anyone else, maybe we could pair on it.
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.
ES module syntax doesn't work completely in gatsby yet. Caused a bit of stress trying to figure that out when i was working on this before because it kinda worked at times, and was confusing.
c0e767d
to
3ca4a58
Compare
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.
@pepopowitz - is it possible to exclude certain pages from results? If so, can we remove the changelog page?
Otherwise, looks great 👍 Merge when ready and we can start looking more closely at how algolia indexes our pages.
I think we can use a |
🚀 PR was released in |
Addresses #633
This PR implements a local plugin hooking up Algolia docsearch. It is based on an external plugin that didn't quite work for our scenario. (Specifically, the search box only worked on initial page render, due to a known issue in that plugin.) I followed the lead of a comment on that issue, but moved all functionality into a local plugin instead of using the external one. We were only using about 12 lines of the external version, and about 30 lines were not working for us, so it felt wrong to keep using the external one.
In action
Things to note in the gif
#__gatsby
at the end, again because it's in the Algolia URL. I'm not sure how it ends up there or how to get rid of it? I think it's probably just another minor annoyance.Outdated information (kept here for posterity)
Original PR body
This PR adds a plugin for hooking up Algolia docsearch. Unfortunately, the plugin doesn't work very well for our scenario - it only connects the search box to Algolia on initial page render. As soon as you start clicking around the site, the search box becomes functionless. This is a known issue in that plugin. It is a result of our search box re-rendering at each new route, something the plugin doesn't account for. I dug a bit to try to fix it within the plugin so I could submit a PR, but I was unsuccessful.Instead, I followed the lead of a comment on that issue. I'm still using the plugin to render the JS and CSS inclusions into the DOM, but connecting the actual search box with an effect in theSearchBox
component.As a result, we're only using about 12 lines of code from the plugin. In addition, we have to provide settings for the plugin to work, but they aren't actually used by the plugin (or the workaround - I don't think I can access the options for another plugin.) My preference at this point would be to ditch the plugin in favor of implementing those 12 lines ourselves, but I'd love to hear your opinions.Remaining work on this PR: