chipzoller / hugo-clarity

A theme for Hugo based on VMware Clarity
Other
571 stars 263 forks source link

Mitigate search results bug #359

Closed rootwork closed 2 years ago

rootwork commented 2 years ago

This PR...

Changes / fixes

This PR addresses the worst of #341, and we could decide that it constitutes a fix. I think the search results highlighting could still be improved, but with this PR it's no longer catastrophically bad nor affecting the site outside of search functions.

This PR ensures that the highlightSearchTerms() function, which is what causes all the extra <span>s and <mark>s, is only run when a query exists; that is only when you click through to a page when performing a search. For most sites (unless most visitors are constantly performing searches) this will mean a big improvement.

Functionally this is pretty basic; it moves findQuery() to the functions.js file so it can be accessed outside of the search function it was in, and then tests if it returns anything before running highlightSearchTerms().

This is a somewhat naive fix, as it simply tests whether a query exists in the URL and applies the highlighting if it does. A site might be using queries for things besides search functions, though. If there's a better way to test "is a search being performed?" @onweru I'm open to ideas.

Screenshots (if applicable)

The bug, and the fix, are not visible when browsing the site unless the end user has made <mark> elements visible even when they are empty. Consequently these screenshots are all of the code that is generated.

Before the fix, on a post where no search has been performed (this is the main bug):

before-nosearch

Before the fix, where a search for "Hugo" has been performed and we've clicked through to that same post:

before-withsearch

After the fix, on a post where no search has been performed (bug fixed):

after-nosearch

After the fix, where a search for "Hugo" has been performed and we've clicked through to that same post (no change):

after-withsearch

Checklist

Ensure you have checked off the following before submitting your PR.

rootwork commented 2 years ago

@onweru Hoping this will be quick to review. As I say above I think the search highlighting could still be improved, but this will address the main bug.

rootwork commented 2 years ago

@onweru Meant to add -- this is more basic than what you suggested in https://github.com/chipzoller/hugo-clarity/issues/341#issuecomment-1194125020 -- if you want to try actually moving the highlightSearchTerms() into the search function itself -- and thus no longer need the search check -- I think that would be fine too.

onweru commented 2 years ago

@onweru Meant to add -- this is more basic than what you suggested in #341 (comment) -- if you want to try actually moving the highlightSearchTerms() into the search function itself -- and thus no longer need the search check -- I think that would be fine too.

There are quite a number of ways to kill a rat. I'm okay with this implementation.