alembic icon indicating copy to clipboard operation
alembic copied to clipboard

[Enhancement] Update site-search.html with relevance sort and highlighted excerpts

Open ao5357 opened this issue 4 years ago • 8 comments

Sacrifices the terse elegance of the original filter callback pattern for result sorting by match count (title matches are boosted by a multiplier) and excerpts that highlight the query string in the page context.

Updated the search index json slightly to include normalize_whitespace, which I've found immensely useful in other applications. (That and compress.html, though that's just an aside).

Left the search index layout front-matter as null instead of none, so that can be resolved separately in #141 .

ao5357 avatar Mar 19 '20 08:03 ao5357

@daviddarnes There's also some GET query string stuff in https://github.com/shelbybrad/shelbybrad.github.io/blob/master/collections/pages/search/search--behavior.js I could include if you're interested. It could remove the need to preventDefault() on submit.

ao5357 avatar Mar 19 '20 08:03 ao5357

Hey @ao5357 👋. Sorry for the delay. This is super cool! Just tried the search locally and it works great. I am interested in your other additions, however I want to keep things fairly simple. Would it be a lot of new JS? If so then maybe the code should be split into it's own js file?

daviddarnes avatar Apr 15 '20 17:04 daviddarnes

@daviddarnes Any such changes would be a bit more substantial, possibly breaking, and definitely dependent on the current PR. If you'd like the alembic search to work more like https://shelbybrad.com/search/ (ie. /search page that operates from a GET parameter) I'll happily take assignment of an issue and throw together a PR.

ao5357 avatar Apr 15 '20 18:04 ao5357

@ao5357 we might get away with it not being breaking, however it would be a real improvement! Would you be comfortable doing that for an open source project? Would be great to have a more refined search feature, especially if the code was approachable by a typical Jekyll type developer 😄

daviddarnes avatar Apr 15 '20 18:04 daviddarnes

@daviddarnes Ok so this latest commit is a bit on the large side. I tried to comment everything that made sense to comment, so theme users can comprehend what's going on in the code.

I added alerts to the Sass, though these are not strictly necessary for the function of the search. It's just a neat feature that adds a bit of color for not a lot of extra code payload.

ao5357 avatar Nov 22 '20 15:11 ao5357

Coming back to these changes I've realised that there's a lot more going on in it than I originally assumed. The PR impacts styles, HTML structure and pretty much changes the entire script. I think this is why I'm reluctant to merge the PR in its entirety. Additionally there's some opinionated changes such as the element selection using attributes over IDs. I intentionally used IDs for selecting elements as I wanted to ensure other elements on the page aren't accidentally selected, as IDs can only be used once on a page.

Would it be possible break down these changes into separate PRs? And the changes reduced so they don't make so much impact on the codebase? For example one could be to only sort the search results if the term appears in a page title. Another PR could be to highlight the matching term in the results.

Sorry to be picky, but large changes like these can make a huge impact on the people using this open source project and I'd like to keep the changes small and incremental. Regardless I really do appreciate you taking the time to contribute! Thank you 😊

daviddarnes avatar Jan 04 '21 13:01 daviddarnes

@daviddarnes The code is there if you decide you want to use it in any manner you deem appropriate. I already provided a more-modest changeset on this PR and then you requested I expand it, only to bait-and-switch me.

ao5357 avatar Jan 04 '21 13:01 ao5357

@ao5357 that's not really what I said, you can see in my first comment that I was "interested in your other additions" you had in mind, but I also said "to keep things fairly simple". I'm not saying these changes aren't worthy, in fact they're greatly appreciated! I'm just trying to work them in without affecting the 1000+ people using this theme right now.

I'll reopen the PR any maybe we can work on pairing this down to just the sorting feature depending on term matching the page title 🙂

daviddarnes avatar Jan 04 '21 16:01 daviddarnes

Closing this because it's over a year old and progress seemed to grind to a halt. The search code isn't great but it works right now, and there are better offerings on the market now that could replace it.

daviddarnes avatar Aug 23 '22 09:08 daviddarnes