[WIP] implement-start-of-search

[kkemple]

This PR begins the work for #135 by adding search support via lunr.js.

Changes:

• Adds a new template for search.json. This is used to build the JSON that needs to be loaded on the client for search.
• Adds a new collection of all definitions for search use.
• Adds two new 11ty filters to properly format search JSON
• Adds a search component in includes
• Adds a scss file for search component
• Adds the search component to index layout

https://deploy-preview-213--selfdefined.netlify.app/

Screen grabs

[tatianamac]

@kkemple Thanks for doing this! This is so awesome. Are you at a point where you'd like feedback yet? If so, what sorta feedback?

(Also I imagine you know this but I find it helpful to include the Deploy preview link: https://deploy-preview-213--selfdefined.netlify.app/)

[kkemple]

Thanks for adding the link! I think biggest piece of feedback I could use is am I doing this the 11ty way? I did some research about how to generate JSON for searching client-side and most examples used this pattern of generating a file from a template you can query from on the client.

Could also use a review of the JS code in 11ty/includes/components/search.njk!

[kkemple]

I've moved the search to a serverless function, currently it has a static json file but that won't scale well, once the search page is actually deployed we can query that from the function. Thanks for all the feedback everyone!

Still left to do:

• make search accessible
• style search to match designs

[tatianamac]

This is so awesome. Thank you, @kkemple ! I am going to remove the Needs Review label off until you're ready for additional feedback (in my mind, waiting for the new approach to include design is okay, but let me know if you feel differently).

[kkemple]

I've updated the serverless function to use the deployment previews /search.json path to get search results instead of using a static file (having a CORS issue, investigating). I've also started to update styles for the search to match the new designs. I still have to:

• run search after search button is clicked instead of onBlur
• need to style search results
• need to make search accessible
  • https://adrianroselli.com/2015/12/responsive-progressive-accessible-vanilla-search.html

Not really ready for review yet but always welcome feedback!

[ovlb]

@kkemple I’ll have a look tomorrow and provide some feedback!

[kkemple]

I've worked more on the search UI and instead of showing the first 100 characters of each search result, it now shows each instance of the search term within a result (including a bit of extra surrounding content for context), as well as highlights the term itself! 💖

screenshot

[mxmason]

Hey @kkemple – I love this! Thank you so much for building it. I like the presentation of each result; especially that I can see the term and the text snippet in which my search can be found. I know that you're still working on the UI, so I write the following with that in mind:

• I would love if each returned term also had a functional link (e.g. clicking "Gaslighting" takes me to that page.
• I would love a count of how many results have been found, so I can understand how much data I am about to parse
• How feasible would it be to make this search result take the user to a new, dedicated results page, in the style of a search engine? If this can be done, it would be a boon for accessibility, because screen reader users would be notified that the browser has navigated, and then they can discover the contents of the page on their own. @tatianamac, how would you feel about this?

Now that I am getting some free time again, I would be glad to support your engineering work here or be a soundboard for accessibility – let me know if I can help!

[tatianamac]

I haven't gotten a chance to be super close to this WIP search but I want to express gratitude for you @kkemple and you @dengeist ! I know this search will function cleverly and be as accessible as possible.

(I also am a fan of dedicated search results pages, bc of accessibility resources EJ pointed out, and also especially if it somehow means that the URL has some sense of permanency, such that you can share it.)

[kkemple]

@dengeist and @tatianamac thank you for the feedback! I'll respond to each in order:

• The term will have a link for sure, just didn't add it yet!
• Love the results count idea! Will add this as well!
• I honestly don't know exactly what would be involved in getting the search results onto its own page. We could definitely navigate to a new url, however, because the site is statically rendered, we would still need to fetch the search results and dynamically create a majority of the results page. I'm not familiar with how a screen reader would handle this. If this is an acceptable solution though, I'm more than happy to make that adjustment!
• I'll also prepare something for when no results are found, because if we do a results page, there is a chance someone will navigate there with no query term.

Would love some feedback on the third point here, I was hoping to stream more work on this today.

[kkemple]

It seems that we could make the area that loads the results on the search page a live region? https://accessibility.huit.harvard.edu/provide-notification-dynamic-changes-content

Not sure if this is the correct approach though.

[kkemple]

Some great updates here! I've moved the search results to a new page so now it can be shared, hopefully is more accessible. Here's a breakdown of current functionality:

• search input on home page now navigates you to the /search page with a query param q to perform the search, example: /search?q=able
• search results page reads the query param and runs the search
• the results are rendered into an aria-live section which should inform screen readers that content has changed and is ready for viewing
• I've added the count in the messaging that is updated when search returns (need to account for no results still)
• The definition titles now link to the definition page

Would love some more feedback on this whenever anyone gets a chance!

[kkemple]

No questions from me, this looks amazing! Thank you for diving in with those awesome fixes! I'm really liking how this is turning out and I'm learning a lot about accessibility! 💖

[ovlb]

Hey, @kkemple,

I finally got around to look at your implementation. It’s really impressive what you’ve built here. Thanks so much for this. I have some remarks, which mostly aim to keep a unified code style throughout the app. I’ll group my feedback in hopefully logical categories. Some of those things might also be on your list already.

HTML & page structure

• Wrap input in a form instead of a section. We have a search form here and should use the appropriate tag. By wrapping it in a form element and adding action and method attributes we’ll have the added bonus that the submit from the arrow to the word «Search»
• Use h3 for search results. Headings should be in order and not skip a level. The live region is an h2, consequentially the results should use h3
• Add a sidebar to allow navigation. See documentation pages where we implemented this structure. Currently, a user is a bit lost on the search page, as it lacks context
• Add the search input to the search page. If a search didn’t yield the result the user was searching for we should let them search again from where they are

Scripting

• Inline scripts. We are using modern JS in an untranspiled environment by having them inline. Moving scripts into dedicated files instead of inlining them should be a goal. I’ve started a bit of work on this in #210, though a dedicated build process is missing there, too. As such, it might be fine to leave it as is until we set this up, but we should have this on our radar.
• The actual search functionality. Is there a possibility to move the code into the repo and use Netlify functions directly instead of pinging the external API?
• Hide search until JS runs. As the functionality is completely JS dependent we should hide the search container and only make it visible once the scripts run. By giving the container element a fixed height we can avoid page jumps.
• Query elements by using .js- classes. Instead of using IDs in querySelector I’d prefer adding scoped classes to the elements, e.g. .js-search-input. This makes it easier to refactor and not accidentally remove an ID that’s needed in a script

Styles

• Reduce specificity. As we are using BEM we can remove the nesting of style rules in _search.scss
• Don’t use px values. To allow users to scale the page all px values should be converted to rem or em. Either directly or by using the pxToRem SASS function
• Focus style on the button. The search button focus style is very subtle right now
• Label styling. The .sub-headline class has styles that are very similar to the label styles. We might use this class for consistency’s sake?

That’s quite a long list and a lot of it comes from me reviewing way too late. If you want help in tackling these issues, I’ll gladly jump in. Also, I don’t want to take anything away from the amazing work you did already. Thanks again and also to @dengeist for reviewing.

O*

[kkemple]

Thanks for all this awesome feedback @ovlb! I've started implementing these changes, I'll go through in order and address each item!

HTML & Page Structure

• converted to a form and was able to remove any js and rely on default form functionality 💖
• I've replaced the h4 headings with h3s
• I was unable to find an example of the sidebar for navigation in any definition pages! I could use some help on this part as well.
• I've added the search component to the search results page

Scripting

• I'm going to leave the script inline until your PR (#210) is merged. This will prevent any merge conflicts that need to be resolved by both of us adding a js folder under assets. If this gets merged before yours, I'll create an issue to track that, or if preferred, I can add that change to your PR.
• I've moved the serverless function into the project so that it can be deployed with Netlify, but it will need a bit more work to be 100% functional (see comments below in Remaining section)
• hiding search is no longer needed as the search form no longer depends on JS 💖
• I've removed all IDs and replaced with classes

Styles

• I've reduced specificity and un-nested styles
• I'm going to leave this item to you! I'm not sure what values should be used or how pxToRem works and feel like you could quickly make these adjustments!
• I would also love if you could handle this one as well, as I'm not sure what you're looking for here and you could probably handle it faster!
• I've removed the styles for label, and used the class you suggested

Remaining

This brings us extremely close to having something we can merge. The last things I can think is making the url in the serverless function an ENV variable so that we can change the url from the deploy preview URL to the production URL. I don't have access to Netlify console so I'm currently unable to do that. Also, it means for search to work properly locally we would need to use the netlify dev command from the Netlify CLI in order to get the serverless function to work correctly.

[colabottles]

Regarding the search form, there is a label issue. The form label is present but not correctly associated with the form. This can be fixed by adding title="Search" to the input field.

[kkemple]

@ovlb I've taken a first pass at adding a table of contents to the search results page. A few things to note:

• The styling is default, and with a search term that has a lot of results or longer words, it looks really wonky (see screenshot below)
• I've added smooth scrolling for when the user clicks on the table of contents and the page updates to the result. This is behind a check for prefers-reduce-motion. (see gif)

screenshot of wonky menu

gif of scrolling behavior

[ovlb]

I’ve pushed the style updates.

Re toc: It might make sense to move the table of content between the number of results and the actual result list to give it more space to breathe. And it’s also in the logical page flow there.

hiding search is no longer needed as the search form no longer depends on JS 💖

But the search page does, or? As such we should still hide search completely for non-JS contexts.

[kkemple]

@ovlb are you saying we should hide search for users that have JS disabled? We can definitely do that. We'll need to add messaging to the search results page as well as those could be shared via URL.

[ovlb]

are you saying we should hide search for users that have JS disabled?

Yip. It’s not the best experience if they are able to submit the form but can’t see the results, imho.

We'll need to add messaging to the search results page as well as those could be shared via URL.

Good point.

[kkemple]

Yeah, totally agree! I'll take this on next time I dive in to work on it!

[ovlb]

@kkemple FYI: I resolved some conflicts stemming from merging #210 into prod, and updated all colours in use to custom properties.

[tatianamac]

@kkemple : I'm doing my sweep through of issues right now, so absolutely no pressure from this message.

How are you feeling about this issue? Are there any blockers that prevent us from merging this in as of right now?