Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Conversation

@markvanlan
Copy link
Contributor

@markvanlan markvanlan commented Dec 16, 2024

This PR adds an "unavailable" state for the AI semantic search toggle. Currently the AI toggle disappears when the sort by is anything but Relevance which makes the UI confusing for users looking for AI results. This should help!

Note there are no specs for this feature. I'm not sure what the LLM extraction mentioned here is, so I didn't touch it.

skip "TODO: Implement test after doing LLM abstraction" do
it "performs AI search in the background and hides results by default" do
visit("/search?expanded=true")
search_page.type_in_search("apple pie")
search_page.click_search_button
end
end

No UI change for regular ol' searching
Screenshot 2024-12-16 at 10 54 56 AM

But when a different sort order is selected, we now have an unavailable state with a tooltip explaining
Screenshot 2024-12-16 at 2 14 41 PM
Screenshot 2024-12-16 at 2 14 37 PM

@xfalcox xfalcox requested a review from keegangeorge December 16, 2024 18:08
Copy link
Member

@keegangeorge keegangeorge left a comment

Choose a reason for hiding this comment

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

Thanks @markvanlan, this looks good! Just a few small comments to address.

About the system spec, yes please feel free to leave as is, I will create a separate todo and follow-up with relevant tests.

I also noticed this PR introduces in a small bug with the text label.

To repro:

  1. Go from quick search to advanced search
  2. Search for “test”
  3. Toggle AI
  4. Append “themes” to search to be “test themes”
  5. Enter to search
    [The text label says: "Press 'search' to begin looking for new results with AI" while AI results are being fetched, and is replaced by "Hiding %{count} results found using AI"]

This should instead be:
["Searching for more results using AI" while AI results are being fetched and then as it is now, replaced by "Hiding %{count} results found using AI"].

Copy link
Member

@keegangeorge keegangeorge left a comment

Choose a reason for hiding this comment

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

Awesome! 👏🏽

@markvanlan markvanlan merged commit 24b1078 into main Dec 16, 2024
6 checks passed
@markvanlan markvanlan deleted the semantic-search-disabled-state branch December 16, 2024 20:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants