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

Add indexing of dropdown entries using Block State #2186

Closed
wants to merge 6 commits into from

Conversation

ictin
Copy link

@ictin ictin commented Jan 31, 2024

The basics

The details

Resolves

Fixes #1940 using the proposed solution from #2004

Fixes

Proposed Changes

The search should find the block and select the dropdown item. Also, both option[0] and option[1] of the dropdown will be indexed.

Reason for Changes

The search fails to find blocks with a dropdown.

Test Coverage

The unit tests have been updated and extended to test for all possible 3-letter combinations as search terms. The new tests have revealed an existing bug, where the last trigram of a word was not indexed. This caused a search for 'ace' for example to fail for "replace". This is fixed in this pull request

Documentation

No update of the documentation is required. Although an remark could be made.

Additional Information

ictin and others added 6 commits January 3, 2024 13:45
Set is not a suitable data structure to store the block State as an object, as it is using the object reference to test for uniqueness. The State is now stringified in the Set, and the set will contain a string version of the State object
…e bug. Fixed bug that didn't indexed the last trigram from a word
@ictin ictin requested a review from a team as a code owner January 31, 2024 12:41
@ictin ictin requested review from maribethb and removed request for a team January 31, 2024 12:41
Copy link

google-cla bot commented Jan 31, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@maribethb
Copy link
Contributor

Hello!

Before I give you an in-depth review, there's a few steps that should be taken:

  1. Please follow the google-cla bot's instructions to sign the CLA. Without this, we will be unable to accept any of your code.
  2. Follow the steps on the "I verified my changes" link to:
    a) Update your PR title to follow conventional commits
    b) Run the linter and fix any warnings or errors that are in this code (you can run npm run lint:fix to autofix some of them)
    c) Run npm run format to run the autoformatter

I'll get back to you with more specific comments soon. Thanks!

@BeksOmega
Copy link
Contributor

Heya @ictin Are you still interested in working on this? if not I'll close it the 23rd

@ictin
Copy link
Author

ictin commented Feb 23, 2024

@BeksOmega I will close it, as I will need to use my corporate email in the commits and pull request to have the CLA. I will close this one and open a clean one

@ictin ictin closed this Feb 23, 2024
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.

Update toolbox-search to search dropdown field options
4 participants