Skip to content

[docs] Introduce "search" feature for website#15916

Closed
jugglinmike wants to merge 1 commit intoweb-platform-tests:masterfrom
bocoup:docs-search-client
Closed

[docs] Introduce "search" feature for website#15916
jugglinmike wants to merge 1 commit intoweb-platform-tests:masterfrom
bocoup:docs-search-client

Conversation

@jugglinmike
Copy link
Copy Markdown
Contributor

I've been reviewing WPT's open "docs" issues and the responses to the 2018 WPT survey, and one of the strongest themes is difficulty locating existing content. We're hoping to improve that by restructuring the documents, and a dedicated text search is another way to help folks find what they're looking for.

This patch implements search on the client, integrating Jekyll output with the Lunr.js JavaScript library. As a MIT-licensed project, I don't think there are any concerns about including it in WPT. For a small client-side library, Lunr.js is surprisingly good at natural language search (e.g. the query "equals" matches the term "equality").

Building the site locally can be a bit tricky, so here are a few screenshots:

2019-03-18-search-inactive

2019-03-18-search-active

2019-03-18-search-results


I also experimented with a simpler solution which relies on DuckDuckGo. While that approach requires less code, it has some drawbacks.

While the visualization of results in this patch could use some work, I wanted to get some feedback before spending more time on it. @gsnedders @jgraham @foolip (+anyone else): does this look promising to you? Do you think DuckDuckGo would be better? Or is there another alternative that I should look in to?

@foolip
Copy link
Copy Markdown
Member

foolip commented Mar 19, 2019

This looks very promising, I was hoping we could have a client-side search just like this! I'll review.

Comment thread docs/assets/lunr.min.js
@@ -0,0 +1,6 @@
/**
* lunr - http://lunrjs.com - A bit like Solr, but much smaller and not as bright - 2.3.6
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need anything in place to remember to update this occasionally?

Comment thread docs/assets/search.js

function getQuery() {
var query = window.location.search.substring(1);
var vars = query.split("&");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

For our audience I think it's OK to depend on https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams/URLSearchParams to simplify this code a bit, i.e., getQuery won't be needed at all.

Comment thread docs/assets/search.js

var appendString = "";

for (var i = 0; i < results.length; i++) { // Iterate over the results
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It'd be fine to use for (var result of results) or results.forEach and not support older browsers if you'd find that nicer and not require explanation.

Comment thread docs/search.html
{% for page in collection.docs %}
"{{ page.url | slugify }}": {
"title": "{{ page.title | xml_escape }}",
"content": {{ page.content | strip_html | jsonify }},
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Aha, xml_escape and strip_html may answer most of the questions I've had. Can you add comments in the suspicious parts of the code pointing out why it's totally safe?

What I'd already written elsewhere:

The text argument here comes from lunr to begin with. Is this text that should be shown verbatim to the user, or already escaped as HTML? In either case it looks like something is missing:

  • escaping of each part of text as it's pieced together if it's verbatim text that happens to include < and such (creating a DocumentFragment with Text and HTMLMarkElement children might be less work)
  • parsing text as HTML and operating on the DOM if text is actually markup (very tedious)

@jugglinmike
Copy link
Copy Markdown
Contributor Author

In gh-16458, we re-implemented the build process to use Sphinx. That includes a client-side search, so this patch is no longer necessary.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants