Navigation Menu

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

Jump to bottom

feat: Improved the Native Search Algorithm #1557

Merged
merged 9 commits into from Mar 8, 2020
Merged

feat: Improved the Native Search Algorithm #1557

merged 9 commits into from Mar 8, 2020

Conversation

ynnelson
Copy link
Contributor

Summary

You can now search tags in addition to H2 and H3 titles as well as the search can now take in multiple words separated by a space.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

I simply improved the suggestions () method in the SearchBox component to now include tags if they exist in the frontmatter, this is combined with the existing behavior of searching through headers. Also, the other improvement is that you can now do a search based on multiple keywords as opposed to only one previously.

This is a very minor adjustment which I think will satisfy a lot of the issues with the native search and allows the user to make pages show up in the search based on the tags assigned to it.

The only change to the UI was to make the suggestion box search sit 0.5 rem lower on the navbar.

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

@ynnelson
Improved the Native Search Algorithm (#1)
242a01e 
* Improved the Native Search Algorithm

You can now search tags in addition to H2 and H3 titles as well as the search can now take in multiple words separated by a space.

* Took out a console.log

* Reflected changes to the SearchBox in the doc
@ynnelson ynnelson mentioned this pull request Apr 24, 2019
Closed
@gevera
Copy link

gevera commented May 14, 2019

This is very helpful. I hope it will get merged soon

@Raiondesu
Copy link
Contributor

+1 for this.

@ulivz
Copy link
Member

ulivz commented Jun 22, 2019 •
edited

Great work, but could you write full tests for it before I decide to approve it?

@ynnelson
Copy link
Contributor Author

ynnelson commented Jul 5, 2019

Pardon my ignorance but I am not really sure how to write full tests for this... could you point me to another test in the repo, that would be helpful in seeing what I need to write up. Thank you. @ulivz

@ynnelson
Copy link
Contributor Author

ynnelson commented Sep 4, 2019

If someone could point me to an example on how to write a test for this, I'd happily do it! @ulivz

@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

It has been a while since you create the pr !

I will have a look about create tests needed @ynnelson thank's for your work

@flozero flozero self-assigned this Sep 5, 2019
@flozero flozero added status: core team assigned Core team member assigned type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress labels Sep 5, 2019
@kefranabg
Copy link
Collaborator

kefranabg commented Sep 12, 2019 •
edited

@ynnelson What you need to do is setup a vue test environment in the VuePress plugin-search. Let me know if you want some help.

@gevera
Copy link

gevera commented Nov 14, 2019

Any progress on this feature?

@kyle-tightest
Copy link

Can someone merge please?

@ynnelson
Copy link
Contributor Author

ynnelson commented Dec 3, 2019

I resolved the conflict, not sure about how to solve the 1 check that failed.

@kyle-tightest
Copy link

@ynnelson there seems to be some linting issues on SearchBox.vue: https://circleci.com/gh/vuejs/vuepress/2803?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link

The other check for "Semantic Pull Request" seems to be due to the title.
I think prefixing the pull request with feat: would solve that.

@kyle-tightest
Copy link

@ynnelson you could also give me write access to your branch and I'll make the required changes.

@ynnelson
Copy link
Contributor Author

ynnelson commented Dec 6, 2019

@kyle-tightest I added you as a collaborator.

@kyle-tightest
Copy link

@ynnelson thanks. Can you please change this pull request's title to feat: Improved the Native Search Algorithm? I dont have access to change the title.

@ynnelson ynnelson changed the title Improved the Native Search Algorithm feat: Improved the Native Search Algorithm Dec 6, 2019
@ynnelson
Copy link
Contributor Author

ynnelson commented Dec 6, 2019

This is ready to be merged i think.

@kyle-tightest
Copy link

@f3ltron

@Royedc4
Copy link

Royedc4 commented Feb 27, 2020

Hey guys this is very relevant, when will this be merged? Cheers!

@flozero
Copy link
Collaborator

flozero commented Feb 27, 2020 •
edited

i am gonna ask to them if they can put it for the next release

@bencodezen bencodezen self-requested a review February 28, 2020 01:34
bencodezen
bencodezen approved these changes Feb 28, 2020
View reviewed changes
Copy link
Member

@bencodezen bencodezen left a comment

Choose a reason for hiding this comment

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

This is a fantastic update. Thank you for doing this!

@sarahdayan
Copy link
Contributor

Hey @ynnelson! As @ulivz said I still think there would be value in testing this. This way, we can fully validate the behavior on our end, and improve the algorithm later on without breaking expectations.

If you extract the logic outside of the component, this should be easy to test. I'm available to help you if you need.

@kefranabg
Copy link
Collaborator

I just added some tests. One more review and it will be ready to be merged 🎉

sarahdayan
sarahdayan approved these changes Mar 7, 2020
View reviewed changes
Copy link
Contributor

@sarahdayan sarahdayan left a comment

Choose a reason for hiding this comment

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

Looks great!

@kefranabg kefranabg merged commit e9fde5c into vuejs:master Mar 8, 2020
@meteorlxy meteorlxy mentioned this pull request Mar 24, 2020
Closed
@matmaxgeds matmaxgeds mentioned this pull request Mar 26, 2020
Closed
@zq99299 zq99299 mentioned this pull request Apr 1, 2020
Closed
1 task
@meteorlxy meteorlxy mentioned this pull request Apr 6, 2020
Merged
10 tasks
@snyk-bot snyk-bot mentioned this pull request May 1, 2020
Open
larionov pushed a commit to larionov/vuepress that referenced this pull request Aug 19, 2020
@ynnelson
feat($plugin-search): improve the native search algorithm (vuejs#1557)
04fae73
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bencodezen bencodezen bencodezen approved these changes

@sarahdayan sarahdayan sarahdayan approved these changes

Assignees

@flozero flozero

Labels
status: core team assigned Core team member assigned type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@ynnelson @gevera @Raiondesu @ulivz @flozero @kefranabg @kyle-tightest @Royedc4 @sarahdayan @bencodezen