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

Implement a search feature #123

Closed
2 tasks done
justinbmeyer opened this issue Oct 7, 2016 · 16 comments
Closed
2 tasks done

Implement a search feature #123

justinbmeyer opened this issue Oct 7, 2016 · 16 comments

Comments

@justinbmeyer
Copy link
Contributor

justinbmeyer commented Oct 7, 2016

Prerequisites:

The implementation of this is to write out the docMap.json somehow ... or maybe a smaller version with just the name, titles, and descriptions of everything in the docMap. Here's where we did this in the past: https://github.com/bitovi/documentjs/blob/b1c5d27afe3fcd5aaaf797417b44af8212016985/site/searchdata.js

When the client loads, it should make a request for that, and use it to search.

As the docMap can still be really big ... 100s of entries, and searching it would be slow ... we'd pre-process some results in previous versions of DocumentJS.

Here's where it happened in the client: https://github.com/bitovi/documentjs/blob/v3.2.0/jmvcdoc/models/search.js#L171

It looks like what we would do is keep a result tree like:

{
  "a": {
     "b": ["abra","abba"],
     "c": ["accent", "acura"]
   },
   "b": { ... },
   ...
}
@mickmcgrath13
Copy link
Contributor

mickmcgrath13 commented Nov 11, 2016

PR: #190
Branch: https://github.com/canjs/bit-docs-html-canjs/tree/123-search

Possible things still to do (needs discussion)

  • Responsive (do we want it to show up on mobile)? no
  • WebWorkers (it isn't immediately obvious to me how to set this up - i've never done it before)
  • fall-through-cache style caching with localStorage (compatible with web workers?)
  • general review of the code (it feels a bit nasty to me for some reason)
  • Move to its own module (separate from bit-docs-html-canjs)?
  • design tweaks (if any)?

@justinbmeyer
Copy link
Contributor Author

For now, move it into its own module within bit-docs-html-canjs.

We might have to indexDB it in a web-worker.

@mickmcgrath13
Copy link
Contributor

For now, move it into its own module within bit-docs-html-canjs.

..maybe a silly question, but what exactly does this mean? Just organize all the files related to search into its own folder instead of intermingled with all of the others?

@justinbmeyer
Copy link
Contributor Author

Instead of being in static/canjs.js <https://github.com/canjs/bit-docs-html-canjs/pull/190/files#diff-8efd6c52b5a29c345f4c332394b874d5>
... make a static/search.js

Justin Meyer
847-924-6039

On Mon, Nov 14, 2016 at 8:41 AM, Mick McGrath notifications@github.com
wrote:

For now, move it into its own module within bit-docs-html-canjs.

..maybe a silly question, but what exactly does this mean? Just organize
all the files related to search into its own folder instead of intermingled
with all of the others?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#123 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEzCtxArhS6-JG4gsbYQxpJJpjUEKi1ks5q-HMWgaJpZM4KRPEO
.

@tomgreever
Copy link
Contributor

tomgreever commented Nov 28, 2016

My suggestion is for the sidebar to display the search results in a simple 2-line format: Page title / snippet preview, perhaps with a count of the number of matches on that page.

  • Searching automatically loads the first / best match
  • Clicking on a result loads that page with that word highlighted on the page (orange)
  • Clearing the search text by clicking X in the search input "resets" the left nav to the regular view while leaving the user on that page.

For example:
screen shot 2016-11-28 at 3 55 47 pm

I also think it would be super cool for it to behave similarly to Command+F on a browser with up/down arrows that can be used to navigate each next result using keystrokes (Enter / Up / Down).

screen shot 2016-11-28 at 3 56 15 pm

@mickmcgrath13
Copy link
Contributor

So we have a decision to make between two types of search. There's Lunr.js (currently [mostly] implemented in this branch), and then there's Algolia (Mentioned here).

For Algolia, there are some questions that we'd need to answer before moving forward with it:

For the Algolia team:

  • If they crawl the site for us, how do they detect changes, or how do we trigger a refresh of the index (we need things to update when we make changes to the docs)?
  • If they crawl, can we specify what is indexed? If so, how?

For Bitovi:

  • Do we want them to crawl, or do we want to write a script using their api?
    • If they do it, potentially less work for us
    • If we do it, it'll be more ready to integrate into bit-docs outside of canjs.com
  • Would we go with the OOTB popover autocomplete functionality, or use the customized design from this issue?

(there are probably other questions to answer, too, like "how do we specify a container for the results to show up in?", but they can probably be answered via the Algolia docs).

I estimate about 1 day's worth of work to finish the Lunr.js implementation (all that remains is to invalidate the localstorage-saved search index when the docs change, and Chasen and I have a good idea for how that should work).

The timeline for the Algolia implementation, on the other hand, is somewhat less clear and will vary depending on the outcomes of the questions above. I think it'd be somewhere in the range of a few days to a week or more depending on how we decide to go about it.

Here is a comparison document that should help in the discussion: Lunr.js vs Algolia

@justinbmeyer
Copy link
Contributor Author

justinbmeyer commented May 15, 2017 via email

@mickmcgrath13
Copy link
Contributor

That's where I was leaning, too

@christopherjbaker
Copy link
Contributor

Lunr, because "Let's get something working" and also because it provides a simpler plugin for others to use. The extra load can be done after the page loads, so as not to interrupt the initial view, or even on-demand. If size were really a concern, the initial load could be just the core items, and an optional checkmark to also load the full items.

@chasenlehara
Copy link
Member

+1 on a solution that gives us full-text search… until then I’ll continue searching our site with site:canjs.com/doc/

@justinbmeyer
Copy link
Contributor Author

Could we use google w/i our site?

@olivernn
Copy link

I'm happy to answer any questions you might have about integrating lunr.

mickmcgrath13 added a commit that referenced this issue May 16, 2017
…torage | some organizational refactors | minor ux tweaks
mickmcgrath13 added a commit that referenced this issue May 16, 2017
…torage | some organizational refactors | minor ux tweaks
mickmcgrath13 added a commit that referenced this issue May 16, 2017
…torage | some organizational refactors | minor ux tweaks
mickmcgrath13 added a commit that referenced this issue May 16, 2017
…torage | some organizational refactors | minor ux tweaks
@mickmcgrath13
Copy link
Contributor

Thanks for the offer @olivernn!

I currently have a branch with Lunr nicely integrated already. You can see it here.

I do have an additional function to format the search term prior to searching which could probably be better utilized as a plugin in the pipeline, but it does work pretty well as-is.

Anyway, feedback is always welcome, but please don't feel obligated.

Thanks, again!

@olivernn
Copy link

@mickmcgrath13 Those links 404 for me...

I think I found the commit that you were referencing though.

You might be able to avoid having to do all that string manipulation for crafting a search string, there is a query method that is more geared towards programatic query building. Lets say you wanted to construct a query for the following search name:foo^10 title:foo^10 description:bar, with a query this would become:

idx.query(function (q) {
  q.term('bar', { fields: ['description'] })
  q.term('foo'. { fields: ['name', 'title'], boost: 10 })
})

Anyway, the rest looks fine, let me know if you run into any issues, or have any more questions.

@mickmcgrath13
Copy link
Contributor

@olivernn Apologies for the 404'd link. I merged & deleted the branch it was pointing to.

Thanks, again, for the offer, and thanks for the tip about the query; I'll give that a shot!

@chasenlehara chasenlehara assigned imaustink and unassigned tomgreever May 22, 2017
imaustink added a commit that referenced this issue May 23, 2017
Fixed search URL prefix bug. Related to #123
@imaustink
Copy link
Contributor

Everything required has been completed and merged to master. Once we make a release, it'll be live.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants