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

no jquery searches #12

Merged
merged 6 commits into from Nov 19, 2017
Merged

no jquery searches #12

merged 6 commits into from Nov 19, 2017

Conversation

andrewscofield
Copy link
Contributor

I didn't have time to write the code for a dropdown, but here is a working js for the input field. I saw the need on issue #2

@jimblue
Copy link

jimblue commented Nov 3, 2017

Amazing 👍 thank you @apotropaic
Can't wait for the merge!

@w00fz w00fz self-assigned this Nov 10, 2017
@w00fz
Copy link
Member

w00fz commented Nov 10, 2017

FYI i'm looking at this today. There are a few issues with this implementation that i noticed and that i want to fix.
I also want to add support for the live URI querystring when in the /search page and as you type in the field.
I will be using @apotropaic's PR as feature to push my changes.

@w00fz
Copy link
Member

w00fz commented Nov 11, 2017

Got the initial rework in:

Pretty much as you had it already @apotropaic, just restructured and hooked up webpack. Also fixed a few issues as well:

  1. support for multiple search/dropdowns per page
  2. close the dropdown when clicking in the body (outside the dropdown)
  3. fixed the minimum count that was hardcoded to 3 rather than ready from the options
  4. have implemented a smarter timer for the keyup which uses lodash throttle

I still need to implement the live URI update on the search page so this PR is still not quite done yet.

Cheers

@andrewscofield
Copy link
Contributor Author

Looks awesome! I was going for a no import solution, but makes sense if you are going to use webpack to just pull in the tools you need, like domready and lodash.

One thought I just had was about using fetch, would it be worth it to include a polyfill for that? I'm sure its got something like 80% coverage, but just a thought.

And when this is merged, we need to update the learn.getgrav.org website asap! I hit enter almost every time I do an advanced search on that site haha

@w00fz
Copy link
Member

w00fz commented Nov 11, 2017

I’m already using GitHub’s fetch as polyfill and handling it via webpack:
https://github.com/trilbymedia/grav-plugin-tntsearch/pull/12/files#diff-7b9b27bb161c35ae42546d7641ee72b4R20

Should be enough for backward compatibility.

Also yes as soon as I’m done with the missing bit I’ll test the learn and make sure we get that updated as well 😄

@w00fz
Copy link
Member

w00fz commented Nov 18, 2017

History has been now implemented. I'm going to be finally merging this PR on monday.
If you want to give it a spin go ahead!

@w00fz w00fz merged commit b0cbb13 into trilbymedia:develop Nov 19, 2017
@w00fz
Copy link
Member

w00fz commented Nov 19, 2017

Merged, thank you so much for your help with this @apotropaic 👍

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.

None yet

3 participants