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

Hotfix: enable site wide search #1370

Merged
merged 1 commit into from
Jan 30, 2023

Conversation

thisisobate
Copy link
Contributor

@thisisobate thisisobate commented Jan 30, 2023

Issue

Lunr depends on whatever we give it as the index to function. In the case of Vitess, we only crawl the summary of each of the docs pages. The summary is usually the first 5 or more sentences of a page hence the rest of the page/doc is not in the index.

Solution

To solve this issue, I created a new field containing all the page contents. This enables a complete site-wide search for content in Vitess.io

Future Improvements

To make the vitess.io search better, we need to improve two things:

  1. Reduce the search results summary to 1 or 2 sentences
  2. Highlight the search keyword in the search result

Action Item

  • Create an issue on Vitess.io for these different search improvements.

Fixes #1355

@netlify
Copy link

netlify bot commented Jan 30, 2023

Deploy Preview for vitess ready!

Name Link
🔨 Latest commit 1e540f9
🔍 Latest deploy log https://app.netlify.com/sites/vitess/deploys/63d75f6df90e820008b34efd
😎 Deploy Preview https://deploy-preview-1370--vitess.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@RobertKielty
Copy link

@thisisobate Thanks for the quick turnaround on this!

On the preview deploy, olap pages are now being found, as are pages with init_db_name_override.

@FancyFane PTAL at https://deploy-preview-1370--vitess.netlify.app/
@orware would love to get your feedback on the workload search

@thisisobate I have a UX nit. Now that the index holds all the content the search functionality has more work to do and there is lag between starting the search and providing the result.

Let's have the search event handler immediately display a spinner animation so that as soon as the search starts, the end-user gets immediate feedback that a search is taking place. What do you think? Doable?

@nate-double-u
Copy link
Collaborator

@thisisobate I have a UX nit. Now that the index holds all the content the search functionality has more work to do and there is lag between starting the search and providing the result.

Let's have the search event handler immediately display a spinner animation so that as soon as the search starts, the end-user gets immediate feedback that a search is taking place. What do you think? Doable?

Is this something we can do in a follow up PR? If the UX update is more than a day's effort, maybe we get the functionality in first and then work to smooth out the UX experience.

@orware
Copy link

orware commented Jan 30, 2023

@RobertKielty The search functionality seemed considerably better at including some of those pages that were previously not being included now in that preview URL so from my end I am seeing an improvement compared to before!

@thisisobate
Copy link
Contributor Author

thisisobate commented Jan 30, 2023

Is this something we can do in a follow up PR?

I agree with you @nate-double-u

Let's tackle this in a follow-up PR.
Thanks for the suggestion @RobertKielty

@FancyFane
Copy link
Collaborator

Also providing feedback, the search functionality is now much better. Doing the search for init_db_name_override yielded ideal search results. Thank you for the quick turn around on this!!

@nate-double-u
Copy link
Collaborator

Let's merge this in as is then, and we can open up a follow up issue.

@nate-double-u nate-double-u merged commit 88037c8 into vitessio:prod Jan 30, 2023
@nate-double-u nate-double-u mentioned this pull request Jan 30, 2023
@deepthi
Copy link
Member

deepthi commented Jan 31, 2023

Thank you @RobertKielty @thisisobate for fixing this! Looking forward to the UX improvement PR 😛

@RobertKielty
Copy link

The following issues were created arising out of doing this work

#1371
#1372
#1376

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

Successfully merging this pull request may close these issues.

[Feedback] Search Functionality
7 participants