Add search functionality #984
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for laughing-payne-b9fbd2 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
This looks great, thanks!
I really like the way A third option would be to use the sidebar, but I think that's easier to miss. |
yndajas
reviewed
Oct 21, 2022
yndajas
reviewed
Oct 21, 2022
yndajas
reviewed
Oct 21, 2022
Merged
Gweaton
force-pushed
the
add-search
branch
3 times, most recently
from
4fa8de5 to
137233b
Compare
Gweaton
force-pushed
the
add-search
branch
4 times, most recently
from
552ad91 to
42ccc46
Compare
yndajas
force-pushed
the
add-search
branch
16 times, most recently
from
dd7d160 to
cfa1d21
Compare
A couple of top-level pages are indexes of links with no Jekyll content - this prevents them from being included in search results (and fixes a bug in loading results containing such pages in the process)
Adds a heading in the format "Showing N results for [search term]"
This brings the code that defines the flow of the IIFE up to the top by putting it in its own `call` method, which is then called at the end (after all the helper methods are defined)
This abstracts the remaining code in the `call` function into declaratively-names methods where possible to make it more understandable at a glance, moving the logic out to helper methods
This replaces the last traditional `for` loop with a `forEach` for easier reading, and also standardises the argument syntax
- Adds a guard clause instead of a big `if... else` block - Cleans up the process of preparing the inner HTML string, formatting this with standard HTML-like indentation
This adds the Standard package and fixes linting errors in `search.js` Standard is the usual choice for dxw repos, although it might be permissible to use any opinionated linter/formatter @see https://github.com/dxw/tech-team-rfcs/blob/main/rfc-035-use-standard-linting-tools-across-all-our-projects.md Per the RFC linked above, we should consider whether to integrate this into CI flows and update the project README
This adds an icon to the right of the search box. It uses a Font Awesome asset and follows a similar style to the buttons that accompany single text input forms on the main dxw.com
Previously if you went to `/search` or `/search?query=` - the search page with no query - it would display a `#` in place of a heading and nothing else. This adds some instructions to get the user back on track
axe DevTools (accessibility) highlighted that there was an empty heading element on the page. The app automatically generates a `h1` for the page title, however we're creating a more dynamic main heading on this page. This cleans up the autogenerated heading after setting our own
If the excerpt starts at the start of the content, ellipses will not be prepended; if the excerpt ends at the end of the content, ellipses will not be appended
Previously only the first match of the exact search query (in full) would be highlighted. This changes the behaviour to highlight all matches of all search terms It also adjusts the `matchIndex` to 100 if there are no matches in the content (this is the case in results where the match is in the page title but not the content). Adjusting the matchIndex to 100 ensures the excerpt in such cases is ~200 characters, rather than ~100 (previously if there was no match, the `matchIndex` equalled `-1`)
Previously the breadcrumbs were underneath the heading in a `div` - this integrates them into the heading and link, bringing the context of each result more to the fore, and reducing the number of lines each result takes up
For improved accessibility, page titles should ideally be unique. This gives the search page a unique title. It doesn't reflect the search terms, but it does at least say "Search - Playbook - dxw" rather than defaulting just to "Playbook - dxw" @see https://dequeuniversity.com/checklists/web/page-title
This removes the default outline of the search box, which had curved edges (going against the Playbook's implicit design principles) and was partially obscured by the search button Instead, since the input and button are tightly coupled (visually and conceptually), the entire search form is outlined when focusing within the search box, with a cleaner design using one of our brand colours. This is the same colour as used for underlining links, so the colour consistently signals a location of (potential) action (clicking a link; searching the playbook) The form outline has a short `ease-in-out` transition on the outline colour and width, but this is skipped if the user prefers reduced motion The default outline is retained on the submit button (when `:focus-visible`) in order to avoid confusion when navigating the site by keyboard. When inside the text input, the text cursor and form outline combine to indicate the focused element, but the submit button doesn't have any analogue to a cursor to help clarify which element within the form is focused, so maintaining the default outline is more important
Previously the `<a>` tags wrapped around the `<h2>` element, which meant that the entire line/blocck was linked. This could result in unintentional opening of links when clicking in whitespace. This moves the `<a>` element inside the `<h2>` element, resulting in only the text being a clickable link
This was referenced Nov 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[edited by Ynda]
Changes
This adds search functionality to the Playbook. The Playbook used to have one very long main page plus a smattering of guide pages, but the Great Playbook Restructure of 2022 (#973) changed all that, so now we have many sections each containing many pages. The benefits of a search feature are therefore much greater now than before
A search box is available in the navbar, top-right on large screens and just below the logo on smaller screens. You can enter one or more terms and hit enter/return or click on the search icon to start a search. The results page will list results by relevance. Each result includes a heading with the breadcrumbs and page title, linking to the page itself, and an excerpt with matches for the search terms highlighted
Preview of the changes
Interactive preview
Try out the changes
Demo
Large screen
Screen.Recording.2022-11-18.at.20.38.34.mov
Small screen
Screen.Recording.2022-11-18.at.20.51.41.mov
Screenshots
Homepage with search box visible
Search results page
Implementation
Design considerations
Breadcrumbs
Midway through development, these were provided under the title with regular body text styling. We felt that they looked a little out of place here, and decided to integrate them into the heading/link
However, we were ultimately undecided on the best approach:
Matches counter
Midway through development, we had a matches counter. Unfortunately the search engine we used doesn't tell you how many matches it found for the search query (or constituent terms), only a score based on a "very perfect scoring mechanism". So we made our own implementation, counting the matches for the entire search query and displaying it under the heading (alongside breadcrumbs at the time)
However, this didn't account for matches in the title, nor for matches of each of the individual search terms when there were multiple. It should be possible to extend the functionality to achieve this, similar to how the match highlighting has been extended, however the results may still be confusing. The "very perfect scoring mechanism" means that the results are not ordered by a simple count, but some more sophisticated measure of relevance. This means that there might be a result that's higher up than a result with a greater number of matches
In the end, we removed the counter, but it's one to think about
Development
The underlying search engine is ElasticLunr. The initial approach was largely adapted from this guide, however this has been extended and refactored quite substantially
Lunr, which ElasticLunr is largely based on, uses the BM25 search result relevance ranking function - ElasticLunr says it has a better mechanism, but presumably it's vaguely similar. Edit: buried in the "Query-time Boosting" section of the ElasticLunr site, there's a link to the actual mechanism: Lucene’s Practical Scoring Function
This PR also adds the Standard package for JavaScript linting (and fixing if you use the VSCode extension or equivalent)
Future work
For future consideration:
standardchecks to the CI