-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: upgrade AutocompleteJS to v1 & update related code (Top Navigation Desktop Only) #1781
Conversation
and remove unused algoliasearch lib
- checks for valid index - adjusts query params based on index
Will using a live view make writing front end tests harder? Is this something we need to mock when rendering component tests? |
@@ -177,7 +177,7 @@ function _fileIcon(hit) { | |||
} | |||
} | |||
|
|||
function _contentIcon(hit) { | |||
export function _contentIcon(hit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit-pick: For consistency this function should not start with an _
if it is being exported
@kotva006 Probably not. Did you see the tests this PR has and do you think another type of test is needed? In this (relatively simple) case, the frontend test operates on HTML that's presumed already-generated from the backend, and assesses whether instantiating the frontend library on it works as expected. And the backend test checks for expected generated markup. And the integration test covers the is-it-present-if-you-go-visit-the-page piece. For a more complex component that leverages more of LiveView's features around handling events and updating state and interacting with the server, I imagine those tests will be more (if not entirely) on the Elixir-side, using various tooling that comes with LiveView. 🤷🏼♀️ |
This will increase the clickable area of links and buttons.
return Object.keys(x).includes("_content_type"); | ||
} | ||
|
||
export const getItemType = (item: Item): string => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function doesn't seem to be used anywhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! will remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks Good
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
Asana Tickets:
Backend highlights
Important
Our search box (both old and new) works by sending a POST request to our
/search/query
endpoint, which in turn does some processing and then encodes and sends a rather complex queries request to Algolia.Before, the shaping the requests data for sending to Algolia was mostly being done with a mess of JS class functions, and finished in the backend in the
SearchController
. Now, this work is all shifted to the backend, to keep the frontend JS simpler. This is mostly achieved though the newAlgolia.Query.Request
module.Note
This PR introduces our first Phoenix-based functional component.
<.algolia_autocomplete />
is intended to be reused wherever we'll need a search box. While it depends on LiveView under the hood to handle instantiation via aphx-hook
object, it's a simpler component with no lifecycle or state. When mounted, it automatically callssetupAlgoliaAutocomplete()
.Frontend highlights
Warning
There's a lot of JS here, but almost all of it is just here to correctly specify the many parameters supported by the autocomplete-js library.
Because of this, I tried to be careful with unit testing. We can assume the library itself is adequately tested, so the tests included here focus on portions that are unique to our particular implementation.
Note
Three "sources" are searchable in the header search box
None of this is newly implemented here, we're reusing endpoints and helper functions and templates as much as possible.
Warning
The autocomplete-js library typings are a bit chaotic.
Dealing with multiple sources (which each return results of different sorts of object) made typing more complicated still. Fighting with TypeScript was a big part of this work; I tried my best!
Visually, I aimed to keep everything as close to our current implementation as possible. I built off the library's default theme to help me achieve that, so there's some custom CSS based on that.
I tweaked the current header navigation and integration testing code to ensure compatibility.
General checks
New UI, or substantial UI changes