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

Update sample search index on changes #125

Merged
merged 2 commits into from
Jun 18, 2024
Merged

Conversation

pkalita-lbl
Copy link
Collaborator

@pkalita-lbl pkalita-lbl commented Jun 12, 2024

Leaving this in draft because I don't want to merge it and update the web version until after Emiley is back from ASM Microbe.


Fixes #91

Summary

The useMiniSearch hook we use to set up a MiniSearch search index in the SampleList component accepts a documents array, but it does not track changes to this list (by design). This means that as the samples list changes (e.g. a new one is added) the search index doesn't follow along. These changes use a useEffect callback to perform a re-indexing whenever the samples list changes.

Separately I noticed as I was updating the tests that we were getting a bunch of spurious warnings (for example). This was resolved by simply updating to the latest versions of @testing-library/react and @testing-library/user-event.

@pkalita-lbl pkalita-lbl marked this pull request as ready for review June 17, 2024 17:07
Copy link
Collaborator

@eecavanna eecavanna 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 to me. Thanks for linking to the "by design" conversation, and for documenting your rationale for using "remove all, add all" in useEffect. I'm comfortable with this being merged in.

@pkalita-lbl pkalita-lbl merged commit a67050e into main Jun 18, 2024
1 check passed
@pkalita-lbl pkalita-lbl deleted the issue-91-new-sample-search branch June 18, 2024 15:36
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.

New sample is not searchable immediately after creation
3 participants