[CT-3469] - Autocomplete for home search bar #119

ben-brooker · 2022-06-29T14:35:15Z

Added autocomplete search to the home page using elastic search and filtered the results based on relevance score. On click, the links search for the content.

Before:

After:

ztsorojev

Left some comments for optimizations you can do! Functionality seems to work well when I try 👍
It's already a big improvement! 🔥

There is just one thing I noticed is that if I type 2 or 3 characters for example, it can show me 1 package suggestion, but if I add a new character, it starts to show me 5 suggestions. Shouldn't it already show 5 before?

Example: plot package, if I write pl or plo, I only get this one package

However if I write plot, I get 5 package suggestions:

pages/index.tsx

ztsorojev · 2022-07-07T20:43:58Z

pages/index.tsx

+  async function autoComplete(query) {
+    try {
+        // fetch the data
+        const resPackages = await fetch(
+          `${API_URL}/search_packages?q=${query}&page=1&latest=1`,
+          {
+            headers: {
+              Accept: 'application/json',
+            },
+          },
+        );
+
+        const { packages } = await resPackages.json();
+        const relevantPackages = packages.filter((p)=>(p.score>2.5));
+        if (relevantPackages.length<5) {
+          setPackageSuggestions(packages.slice(0, relevantPackages.length));
+        }
+        setPackageSuggestions(relevantPackages.slice(0,5));
+
+        const resFunctions = await fetch(
+          `${API_URL}/search_functions?q=${query}&page=1&latest=1`,
+          {
+            headers: {
+              Accept: 'application/json',
+            },
+          },
+        );
+        const { functions } = await resFunctions.json();
+        const relevantFunctions = functions.filter((f)=>(f.score>10));
+        if (relevantFunctions.length<5) {
+          setFunctionSuggestions(functions.slice(0, relevantFunctions.length));
+        }
+        setFunctionSuggestions(functions.slice(0,5));
+
+    } catch (err) {
+      console.error(err);
+    }
+  }


Can this live in Autocomplete.tsx?
So that the component handles all the logic itself

pages/index.tsx

ztsorojev · 2022-07-07T20:52:48Z

pages/index.tsx

+        const resFunctions = await fetch(
+          `${API_URL}/search_functions?q=${query}&page=1&latest=1`,
+          {
+            headers: {
+              Accept: 'application/json',
+            },
+          },
+        );


Can you do a Promises.all so that both queries are sent at the same time and you don't have to wait for one to be finished?

You can do something like

Suggested change

const resFunctions = await fetch(

`${API_URL}/search_functions?q=${query}&page=1&latest=1`,

{

headers: {

Accept: 'application/json',

},

},

);

const [resPackages, resFunctions] = await Promises.all([

fetch(...),

fetch(...)

]);

You can read about the difference between using multilple await and Promise.all here: https://stackoverflow.com/questions/45285129/any-difference-between-await-promise-all-and-multiple-await

ztsorojev · 2022-07-07T20:54:17Z

pages/index.tsx

+        const { functions } = await resFunctions.json();
+        const relevantFunctions = functions.filter((f)=>(f.score>10));
+        if (relevantFunctions.length<5) {
+          setFunctionSuggestions(functions.slice(0, relevantFunctions.length));
+        }
+        setFunctionSuggestions(functions.slice(0,5));


Btw I think it's more clear to call it topics and not functions since that's what we already use it everywhere in the codebase (UI, API, db)

ztsorojev · 2022-07-07T20:55:17Z

pages/index.tsx

+        if (relevantFunctions.length<5) {
+          setFunctionSuggestions(functions.slice(0, relevantFunctions.length));
+        }
+        setFunctionSuggestions(functions.slice(0,5));


Suggested change

if (relevantFunctions.length<5) {

setFunctionSuggestions(functions.slice(0, relevantFunctions.length));

}

setFunctionSuggestions(functions.slice(0,5));

setTopicSuggestions(relevantTopics.slice(0, Math.min(relevantPackages.length, 5));

ztsorojev · 2022-07-07T20:56:52Z

pages/index.tsx

+        );
+
+        const { packages } = await resPackages.json();
+        const relevantPackages = packages.filter((p)=>(p.score>2.5));


This is typescript, so you can use optional chaining in places where you are not sure if the variable you are accessing is defined or not.

Suggested change

const relevantPackages = packages.filter((p)=>(p.score>2.5));

const relevantPackages = packages?.filter((p)=>(p?.score>2.5));

Left some comments for optimizations you can do! Functionality seems to work well when I try 👍 It's already a big improvement! 🔥

There is just one thing I noticed is that if I type 2 or 3 characters for example, it can show me 1 package suggestion, but if I add a new character, it starts to show me 5 suggestions. Shouldn't it already show 5 before?

Example: plot package, if I write pl or plo, I only get this one package

However if I write plot, I get 5 package suggestions:

I think this is because I set the limits on the elasticsearch for packages the score defaults to 1.82... if the results are a low match. In order for the score to go above this number, the query has to be quite similar. (see picture below).
I can either:

Set the filter for the scores so that irrelevant results don't come up. This will mean that there are sometimes no suggestions

Don't filter the results so that there are always suggestions but they might be irrelevant

ztsorojev · 2022-07-07T20:57:35Z

components/Autocomplete.tsx

+          </div>
+          <div>
+            {
+              functionSuggestions.length>0
+              &&
+              searchInput
+              &&
+              <ul>
+              <li className="my-2 ml-2 pl-4 text-dc-grey200 flex justify-between">
+                <Heading as="h3" size={300}>FUNCTIONS</Heading>
+              </li>


Can you add some space between the the sections?
It will look better 👍

ztsorojev · 2022-07-07T20:59:22Z

components/Autocomplete.tsx

+    function autoCompleteSearch(query) {
+      router.push(`/search?q=${encodeURIComponent(searchInput)}`);
+    };


You can rename it so it's more clear, it can even be called onClick tbh

ztsorojev · 2022-07-07T21:01:21Z

components/Autocomplete.tsx

+    packageSuggestions: Array<any>,
+    functionSuggestions: Array<any>


In my opinion you shouldn't need these props, the component should handle them itself.

There are actually two approaches:

Move all the logic to Autocomplete.tsx so that everything is handled in one place. Everyone who imports Autocomplete will get a search bar that just works.

Make Autocomplete a wrapper component that handles all the logic. Then inside it, you have a child component that is basically how you wrote the current Autocomplete component: it takes props and just renders them

I agree, I decided to move all of the logic into Autocomplete

ztsorojev · 2022-07-11T14:23:16Z

pages/index.tsx

+  useEffect(() => {
+  }, [searchInput])


We can remove this

ztsorojev · 2022-07-11T14:23:37Z

pages/index.tsx

@@ -1,13 +1,15 @@
 import { GetServerSideProps } from 'next';
 import { useRouter } from 'next/router';
-import { useState } from 'react';
+import { useEffect, useState } from 'react';


Remove the useEffect in that case

ztsorojev · 2022-07-11T14:26:02Z

components/Autocomplete.tsx

+                      key={p.fields.package_name}
+                      onClick={()=>onClick(p.fields.package_name)}
+                      className="flex items-center px-4 py-2 cursor-pointer hover:bg-dc-beige200 hover:opacity-0.5"
+                      >
+                        <Paragraph className="pl-2 text-lg">{p.fields.package_name}</Paragraph>


Add optional chaining here just in case

ztsorojev · 2022-07-11T14:26:20Z

components/Autocomplete.tsx

+                      key={t.fields.package_name+t.fields.name}
+                      onClick={()=>onClick(t.fields.name)}
+                      className="flex items-center px-4 py-2 cursor-pointer hover:bg-dc-beige200 hover:opacity-0.5"
+                      >
+                        <div>
+                          <Paragraph className="px-2 text-lg">{`${t.fields.name}`}</Paragraph>
+                        </div>
+                        <div>
+                          <Paragraph className="text-lg text-dc-red">{`(${t.fields.package_name})`}</Paragraph>


here too, optional chaining

ben-brooker requested a review from ztsorojev June 29, 2022 14:35

ben-brooker force-pushed the bb/search-autocomplete branch 2 times, most recently from e70f64d to f392311 Compare July 1, 2022 12:32

ben-brooker changed the title ~~feat: base autocomplete functionality~~ [CT-3469] - Autocomplete for home search bar Jul 1, 2022

ben-brooker marked this pull request as ready for review July 1, 2022 13:00

ben-brooker requested a review from Mim1991 July 1, 2022 13:07

ztsorojev reviewed Jul 7, 2022

View reviewed changes

ben-brooker requested a review from ztsorojev July 8, 2022 18:50

feat: add autocomplete component and connect to home search bar

beef2f3

ben-brooker force-pushed the bb/search-autocomplete branch from 311e7ec to beef2f3 Compare July 8, 2022 18:58

ztsorojev reviewed Jul 11, 2022

View reviewed changes

pages/index.tsx Outdated

Comment on lines 26 to 27

useEffect(() => {

}, [searchInput])

Copy link

Contributor

ztsorojev Jul 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove this

ztsorojev reviewed Jul 11, 2022

View reviewed changes

ben-brooker added 2 commits July 11, 2022 15:30

chore: pr changes to autocomplete

d2ee5ed

chore: optional chaining

d4c6ace

ben-brooker merged commit bd0936d into main Jul 11, 2022

ben-brooker deleted the bb/search-autocomplete branch July 11, 2022 15:05

	const relevantPackages = packages.filter((p)=>(p.score>2.5));
	const relevantPackages = packages?.filter((p)=>(p?.score>2.5));

		packageSuggestions: Array<any>,
		functionSuggestions: Array<any>

[CT-3469] - Autocomplete for home search bar #119

[CT-3469] - Autocomplete for home search bar #119

Uh oh!

Conversation

ben-brooker commented Jun 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ztsorojev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ben-brooker commented Jun 29, 2022 •

edited

Loading

ztsorojev left a comment •

edited

Loading