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

feat(default-layout): sortable search results, search phrases, experimental config #3652

Merged
merged 31 commits into from
Sep 22, 2022

Conversation

robinpyon
Copy link
Contributor

@robinpyon robinpyon commented Sep 15, 2022

findability-2

Description

Features

  • Order search results by relevance, created and last updated date.
  • Search for documents containing an exact phrase by wrapping text inside double quotes.
  • Add __experimental_omnisearch_visibility: true to your document schemas to hide them from omnisearch. Perfect for hiding workflow-related documents or configuration singletons that don’t need to be accessed frequently.
  • View relevance scores for search results via a debug flag (#_debug_search_score)

Bug fixes

  • Fixes an issue where the scroll position of search results wasn’t being correctly retained in some instances.
  • Fixes an issue where search would cause the studio to crash if Local Storage is unavailable.

What to review

Notable changes

  1. Taking cues from the reference and cross dataset reference input components – the function to generate weighted search is now colocated within default-layout rather than being an export of @sanity/base/search

    • Similar to reference / cross reference input components, this now imports from @sanity/base/_internal
    • Given that part:@sanity/base/search and part:@sanity/base/search/weighted are not being used anywhere else in the studio, they’ve been removed
  2. Logic for how we extract search queries and calculate scores has been expanded to accommodate searching for phrases in wrapped quotes:

    • Fundamentally, this is handled in a very similar way to how search worked prior. My understanding of search, and how phrases come into play can be found here (internal doc)
    • The important thing to note is that how we handle non-quoted text searches hasn’t changed. If you don’t engage with this feature then you won’t notice a difference here.
  3. A few more comments are now attached with each search query for measurement. Full example:

    // findability-mvi:2
    // findability-recent-search:4
    // findability-selected-types:2
    // findability-sort:relevance

    In the above example, findability-recent-search:4 indicates that this was the 5th item clicked in recent searches. All these comments will be removed once client-side instrumentation is in place.

  4. In the test studio: a debug schema experimentalOmnisearchVisibilityTest has been added. These documents have __experimental_omnisearch_visibility:false and won’t appear in omnisearch.

  5. Sorting by relevance will now sort by _updatedAt desc in GROQ before applying scoring on the client, leaning towards more recent documents in general. Previously, omnisearch was sorting on _id asc.

    • Search within reference + cross dataset reference fields are unchanged, and will continue to sort on _id asc.
  6. Notable refactoring:

    • Simplified createWeightedSearch signature slightly, and have moved more into the SearchOptions type
    • Some unused dev dependencies were removed from default-layout (mocha, nyc) and it now uses react-testing-library .
    • default-layout was also added to the root level jest.config.js
    • Search related tests in @sanity/base are now colocated and have been expanded on slightly.
    • Quite a bit of code was touched to ensure this is valid once we can enable strictNullChecks: true.

Things to test

  1. Change the sorting of search results:
    • Does this work and make sense to you? Bearing in mind scoring isn’t taken into consideration when not sorting on relevance.
  2. Search on phrases with quoted text:
    • View relevance scoring in debug mode – do these scores seem reasonable to you?
    • Try search for strings with special characters – some good candidates to compare and contrast (test studio):
      1. "winnie-the-pooh" vs winnie-the-pooh
      2. "go, team. go!" vs go, team. go!

Notes for release

Highlights

Search updates

findability-2

  • Order search results by relevance, created and last updated date.
  • Search for documents containing an exact phrase by wrapping text inside double quotes.

Bugfixes

  • Fixes an issue where the scroll position in global search wasn’t being correctly retained in some instances.
  • Fixes an issue in global search that would cause the studio if Local Storage is unavailable.

@robinpyon robinpyon added the v2 label Sep 15, 2022
@vercel
Copy link

vercel bot commented Sep 15, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
studio-workshop ✅ Ready (Inspect) Visit Preview Sep 20, 2022 at 0:54AM (UTC)
test-studio ✅ Ready (Inspect) Visit Preview Sep 20, 2022 at 0:54AM (UTC)

@robinpyon robinpyon requested a review from snorrees September 15, 2022 23:24
Copy link
Contributor

@snorrees snorrees left a comment

Choose a reason for hiding this comment

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

Looks good, thorough and well explained as always :D

Just found some minor nitpick and open questions for myself.

Testing:

  • Seems to work like I would expect
  • As I mentioned elsewhere, the bug where preview does lags behind document updates confused my testing a bit. But this is not at all related to this code, and will be fixed when that issue is adressed. I also dont think preview not updating in search result is really a big problem as you typically wont be searching for already opened documents.
  • Scores: Unless you know the underlying algorithm, these will probably feel a bit random to people. Title * 10, subtitle * 2, partial match / 2, ect ect. Sorting wise it feels good though!

offset: pageIndex * SEARCH_LIMIT,
options: {
// Comments prepended to each query for future measurement
comments: [
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider extracting the comments constructuin and adding some unit tests. Since its invisible to the user, we might not pick up if we break it (for instant if isRecentSearchTerms is changed without thinking about this usage).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would love a bit more direction here on specific tests you'd expect!

Right now, comments in search options has type safety, and there's a test for createSearchQuery that (loosely) covers prepended comments when creating queries

@robinpyon
Copy link
Contributor Author

Thanks @snorrees!

Re: scores: I agree! Especially with how search will use preview fields (which can be automatically inferred if you don't have a preview configuration) .... mix in the interplay between __experimental_search and it can all seem a bit much. It begs the question of whether this needs to be surfaced in documentation more.

Copy link
Member

@rexxars rexxars left a comment

Choose a reason for hiding this comment

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

This looks great. I do worry about potentially breaking someone's code by removing the search parts, however.

…ilst open

Callbacks for `useClickOutside` were triggering even when the search popover was closed, unnecessarilly resetting
the position of the results virtual list.
…n overflow containers receive focus via drag handles
…g various interactions: panel unmount, recent search click and on clear
…es during various interactions: panel unmount, recent search click and on clear"

This reverts commit 151241c.
…until both popover + dialog components have mounted
…e search specs (plural) to be consistent with typing
…f search results when ordering by custom fields
…nd GROQ comments in createSearchQuery, group tests
… define scoped weightedSearch function in default-layout
@robinpyon robinpyon merged commit d60e29e into next Sep 22, 2022
@robinpyon robinpyon deleted the findability-2 branch September 22, 2022 19:50
bjoerge pushed a commit that referenced this pull request Sep 23, 2022
…mental config (#3652)

This commit updates search with the following features:
- Order search results by relevance, created and last updated date.
- Search for documents containing an exact phrase by wrapping text inside double quotes.
- Use the experimental config `__experimental_omnisearch_visibility` to hide documents from global search.
- Display relevance scores in search results via a debug flag (`#_debug_search_score`)

It comes with the following fixes:
- Fixes an issue where the scroll position of search results wasn’t being correctly retained in some instances.
- Fixes an issue where search would cause the studio to crash if Local Storage is unavailable.
robinpyon added a commit that referenced this pull request Oct 19, 2022
…onfig (#3764)

This commit is a V3 port of #3652 

It updates search with the following features:
- Order search results by relevance, created and last updated date.
- Search for documents containing an exact phrase by wrapping text inside double quotes.
- Use the experimental config `__experimental_omnisearch_visibility` to hide documents from global search.
- Display relevance scores in search results via a debug flag (`#_debug_search_score`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants