@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/)

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!

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

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).

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!

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

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! 💖

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!

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 (🌓 Dark Mode #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.

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.

@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)

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.

@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.

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.

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

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

@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?