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

[documentation] Add a nice search experience to the website #416

Merged
merged 5 commits into from Sep 5, 2018

Conversation

s-pace
Copy link

@s-pace s-pace commented Aug 28, 2018

This PR will add DocSearch to the documentation website. It will allow an user to have a learn-as-you-type experience by displaying results thanks to a dropdown in a live way.

If there is no result, pressing enter will enable the legacy search.

Ref Bootstraping PR as jquery/api.jquery.com#1104

This commit will add DocSearch to the documentation website. It is the first step in order to fully integrate it.

Ref Bootstraping PR as jquery/api.jquery.com#1104
@jsf-clabot
Copy link

jsf-clabot commented Aug 28, 2018

CLA assistant check
All committers have signed the CLA.

@s-pace
Copy link
Author

s-pace commented Aug 28, 2018

You can wait for our green light.

We will do an internal check in order to provide you a final PR soon

Sylvain Pace added 2 commits August 29, 2018 15:57
This commit will enhance the includes by moving it to the end of the page
@s-pace
Copy link
Author

s-pace commented Aug 29, 2018

On iPhone X:
iPhone X

On iPad:
iPad

On desktop:

desktop

Live demo of this PR

Please review it :)

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

A few remarks, otherwise looks good.

One thing I couldn't comment inline - please don't add the .DS_Store file to the repository.

indexName: 'jquery',
inputSelector: 'input[name=\'s\']',
debug: true // Set debug to true if you want to inspect the dropdown
})" async></script>
Copy link
Member

Choose a reason for hiding this comment

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

We use tabs for indentation in this file, can you update it? Also, what's inside of the object should be indented more than the closing parens.

Copy link
Author

Choose a reason for hiding this comment

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

Sure! Done

@@ -0,0 +1,16 @@
/* Custom DocSeach CSS to adapt the generic one * See https://community.algolia.com/docsearch/styling.html for more info */
nav#main .searchform {
text-shadow: none;
Copy link
Member

Choose a reason for hiding this comment

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

Please use tabs, not spaces.

@s-pace
Copy link
Author

s-pace commented Sep 5, 2018

Thank you for the guidance @mgol.
Sorry for the .DS_Store, removed ✅
Let us know if you need anything else

@@ -0,0 +1,19 @@
/* Custom DocSeach CSS to adapt the generic one * See https://community.algolia.com/docsearch/styling.html for more info */
nav#main .searchform {
text-shadow: none;
Copy link
Member

Choose a reason for hiding this comment

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

This file still uses spaces, please change to tabs.

Copy link
Author

Choose a reason for hiding this comment

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

My bad. fixed

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

@mgol mgol merged commit 495f31a into jquery:master Sep 5, 2018
@mgol
Copy link
Member

mgol commented Sep 5, 2018

I've tested it now on https://api.jquery.com/ and it works absolutely awesome; such a nice experience. Wonderful work, thanks a lot! 👏

@dmethvin
Copy link
Member

dmethvin commented Sep 5, 2018

THIS IS AWESOME! 🎉 Many thanks @s-pace, and @mgol for getting this landed. Much better than the old search.

@s-pace s-pace deleted the adding_docsearch branch April 2, 2020 14:05
mgol pushed a commit that referenced this pull request Apr 20, 2020
This commit adds DocSearch to the UI/Mobile/QUnit websites. It allows
a user to have a learn-as-you-type experience by displaying results
thanks to a dropdown in a live way.

If there is no result, pressing enter will enable the legacy search.

The commit also fixes the jQuery DocSearch handling to be skipped
when the search input cannot be found.

Ref #416
Closes #421
@Krinkle
Copy link
Member

Krinkle commented Aug 6, 2020

@s-pace Hi - is the API key added here part of an account we can access? The QUnit since has since switched to a static site where we push new content with a secret key during CI builds (ref qunitjs/qunit#1460). As I wasn't able to find which account this key belongs to (e.g. to login and generate a secret key), we opted to create an ad-hoc personal account for now and use. However, I'd rather this not depend on a personal account long-term.

What do you recommend we do?

@s-pace
Copy link
Author

s-pace commented Aug 10, 2020

👋 @Krinkle
Please apply to DocSearch from this application form and we will make sure to grant you access to these analytics. The apiKey is not related to any account and it is a search only one that can be safely shared publicly.

@Krinkle
Copy link
Member

Krinkle commented Aug 10, 2020

@s-pace I don't think we need/want analytics. I'm referring to the secret key used in jekyll-algolia to push new content after an update. This is currently set jquery/qunit's Jenkins job in a secret ALGOLIA_API_KEY environment variable for post-commit builds on the main branch. These are generated from the logged-in Algolia account which is currently a personal one.

Would you recommend we just create another account like that, but with shared credentials stored in a safe place by OpenJS Foundation? Note that QUnit doesn't use docsearch.js, I don't know if what we use falls under "DocSearch", though.

s-pace pushed a commit to algolia/docsearch-configs that referenced this pull request Aug 11, 2020
@s-pace
Copy link
Author

s-pace commented Aug 11, 2020

Thanks for the details. I thought that QUnit was still using DocSearch. I have disabled the daily crawl since this index is not used.

The account you are referring to is tied to @trentmwillis account. I would definitely recommend you to create a shared account with shared credentials stored in a safe place by the OpenJS foundation.

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

Successfully merging this pull request may close these issues.

None yet

5 participants