Skip to content
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

Implement Orama search (again) #115

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

brookslybrand
Copy link
Contributor

@brookslybrand brookslybrand commented Jul 17, 2024

Staging Site

I want to a/b test this new search tool and look at the data of both after they've sat there for a bit of time. Also hoping anecdotally we get some feedback.

The a/b testing is captured in a cookie-session that will expire after a week. You can however force yourself into a bucket by adding either of the following search params:

This is a continuation of #104

@brookslybrand
Copy link
Contributor Author

For some reason hitting enter/return doesn't trigger a navigation. I think this might be a bug with the orama search box

broken-keyboard-accessibility.mov

@micheleriva
Copy link

on it!

app/modules/orama-search.tsx Outdated Show resolved Hide resolved
app/modules/orama-search.tsx Outdated Show resolved Hide resolved
@brookslybrand
Copy link
Contributor Author

I looked into the keyboard issue further, and it's only when using tabs that hitting "enter" doesn't trigger a navigation. If I use arrow keys it works fine

tabs-no-work.mov

@rjborba
Copy link

rjborba commented Jul 19, 2024

Hey @brookslybrand, how're doing? I'm Rodrigo from Orama.

We have upgraded our Searchbox version with a HOTFIX. Would you mind to apply this diff to your branch?

Thanks!

diff --git a/package.json b/package.json
index c6de7c0..dfc4b7d 100644
--- a/package.json
+++ b/package.json
@@ -22,7 +22,7 @@
   "dependencies": {
     "@docsearch/css": "^3.6.0",
     "@docsearch/react": "^3.6.0",
-    "@orama/searchbox": "^1.0.0-rc50",
+    "@orama/searchbox": "^1.0.0-rc52",
     "@oramacloud/client": "^1.3.6",
     "@remix-run/node": "2.8.1",
     "@remix-run/react": "2.8.1",

@brookslybrand
Copy link
Contributor Author

Thanks @rjborba! I never saw your message, but wanted to let you know I did upgrade and that version is working. Thanks!

@brookslybrand
Copy link
Contributor Author

It seems like the focus trap for the modal is not working in Safari for some reason. I can't recreate this issue on Chrome

focus-trap.mov

@rjborba
Copy link

rjborba commented Jul 22, 2024

Thanks for sharing @brookslybrand! We're going to take a look!

@brookslybrand
Copy link
Contributor Author

Awesome, thank you @rjborba!

Another little one: when I use the arrow keys, it doesn't automatically scroll when I get past the 3rd item. It'll scroll if I do an up arrow key, but it'd be ideal if it scrolls to whatever I'm focused on so I don't lose my place.

Also, and I'm not sure if this is something I can easily customize, the focus color (at least for light mode) is pretty hard to distinguish. I am slightly color deficient, so it may be more pronounced for me

no-scroll.mov

@rjborba
Copy link

rjborba commented Jul 23, 2024

Awesome, thank you @rjborba!

Another little one: when I use the arrow keys, it doesn't automatically scroll when I get past the 3rd item. It'll scroll if I do an up arrow key, but it'd be ideal if it scrolls to whatever I'm focused on so I don't lose my place.

Also, and I'm not sure if this is something I can easily customize, the focus color (at least for light mode) is pretty hard to distinguish. I am slightly color deficient, so it may be more pronounced for me

no-scroll.mov

@brookslybrand all very relevant observations. We will be releasing a version addressing all these bullets as well.

I will drop a message as soon as we have it. Thanks! :)

@rjborba
Copy link

rjborba commented Jul 24, 2024

@brookslybrand Hi! Quick update so you guys can be up and running

Another little one: when I use the arrow keys, it doesn't automatically scroll when I get past the 3rd item. It'll scroll if I do
an up arrow key, but it'd be ideal if it scrolls to whatever I'm focused on so I don't lose my place.

The scroll issue is fixed. We're going to publish a new version ASAP. I'll drop a message when we do it.

It seems like the focus trap for the modal is not working in Safari for some reason. I can't recreate this issue on Chrome

This one is actually a limitation with Safari:
"By default tab-access is disabled in safari. To enable it, check "Preferences > Advanced > Press tab to highlight each item on a page"

image

It looks like there ins't much we can do about it. I'll look into it a bit further to be sure.

Also, and I'm not sure if this is something I can easily customize, the focus color (at least for light mode) is pretty hard to distinguish. I am slightly color deficient, so it may be more pronounced for me

About this stying-related question, we work with css vars which you can override with whatever you need.
In this case, the token --background-color-fourth is used on the background of selected item. You can replace it with whatever you think is suitable!

diff --git a/app/modules/orama-search.tsx b/app/modules/orama-search.tsx
index ba5b4d4..69d61c5 100644
--- a/app/modules/orama-search.tsx
+++ b/app/modules/orama-search.tsx
@@ -40,6 +40,14 @@ export function SearchModalProvider({
             onClose={() => setShowSearchModal(false)}
             colorScheme={colorScheme}
             theme="secondary"
+            themeConfig={{
+              light: {
+                "--background-color-fourth": "red"
+              },
+              dark: {
+                "--background-color-fourth": "green"
+              }
+            }}
             resultsMap={{
               description: "content",
             }}

As I said before, we're going to publish a new version fixing the scroll. Meanwhile please feel free to raise any question/bug. Thanks!

@rjborba
Copy link

rjborba commented Jul 24, 2024

Hey @brookslybrand!
We just published a new version of our SearchBox.

Please upgrade yours to 1.0.0-rc53

diff --git a/package.json b/package.json
index f6f9a1d..da94486 100644
--- a/package.json
+++ b/package.json
@@ -22,7 +22,7 @@
   "dependencies": {
     "@docsearch/css": "^3.6.0",
     "@docsearch/react": "^3.6.1",
-    "@orama/searchbox": "^1.0.0-rc52",
+    "@orama/searchbox": "^1.0.0-rc53",
     "@oramacloud/client": "^1.3.6",
     "@remix-run/node": "2.8.1",
     "@remix-run/react": "2.8.1",

Is has a fix related to scroll to view for result elements.

@brookslybrand
Copy link
Contributor Author

Awesome, thank you @rjborba for the fix, instructions, and explanation!

I've pushed the latest. Going to work on an A/B testing strategy next


// if no bucket in the session, assign the user
if (!isBucketValue(bucket)) {
bucket = Math.random() > 0.5 ? "orama" : "docsearch";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BEFORE MERGING: Change this to 0.1

Copy link
Member

Choose a reason for hiding this comment

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

BEFORE MERGING ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants