Skip to content

ETT-1529: bugfix for clearing input when switching from collection to website search#149

Merged
carylwyatt merged 3 commits into
mainfrom
ETT-1529-searchbar-bug
Jun 11, 2026
Merged

ETT-1529: bugfix for clearing input when switching from collection to website search#149
carylwyatt merged 3 commits into
mainfrom
ETT-1529-searchbar-bug

Conversation

@carylwyatt

@carylwyatt carylwyatt commented Jun 9, 2026

Copy link
Copy Markdown
Member

The problem

On prod, if you start from the "website" search option, type in a keyword, then switch back to "collection" search, your search input gets erased.

The fix

Most of the search logic is inside the $effect block, and all the variables inside that block are reactive. When the dropdown select changes value (from "Website" to "Collection"), everything in the effect block runs, including setting the search input to a blank string. The fix ended up being very simple: check to make sure there isn't already a value in the input before resetting it to an empty string.

I also added a guard to the top of the $effect block. I was getting an error in storybook that some values weren't defined, so I added the check to make sure the UI was rendered before trying to do anything else.

Testing

This is staged on dev-3: https://dev-3.www.hathitrust.org/ (please excuse the ACF Better Search warnings at the top of the page... this plugin was finally updated last week, and I will be updating the plugin soon!)

  1. In the search bar, use the dropdown to switch from "Collection" search to "Website" search.
  2. Switch back to "Collection."
  3. Your keywords will be the same!

I tested this in all of the places the search bar appears (wordpress, ls, mb, catalog, pt) and had no issues, but feel free to test as much as you want!

Automated test

I added a storybook test that checks for this bug: https://dev-3.babel.hathitrust.org/common/firebird/dist/storybook/?path=/story/search-bar--type-then-change-fields&globals=viewport:bsLg

@carylwyatt carylwyatt force-pushed the ETT-1529-searchbar-bug branch from e8157b1 to b047717 Compare June 9, 2026 00:56
},
};
export const Desktop = {
args: {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I removed this args block. The search bar doesn't have a modal, so I don't know why this was here to begin with.

@carylwyatt carylwyatt requested a review from eumalin June 9, 2026 19:06
@eumalin

eumalin commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

This looks like a thorough solution and testing. I tested some parts and they work for me too. Great work!

await canvas.getByLabelText('Search using keywords').focus();
await userEvent.type(canvas.getByLabelText('Search using keywords'), 'elephant');
const whereToSearch = canvas.getByLabelText('Where do you want to search?');
await userEvent.selectOptions(whereToSearch, 'Collection');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does this catch the bug if the default is already "Collection"? I think we need to switch to "Website" first so the effect actually runs the buggy branch, otherwise this passes even without the fix.

maybe we could add await userEvent.selectOptions(whereToSearch, 'Website'); before switching back.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It's hard to see in this frame because the interaction tests already ran, but the default Desktop view starts with Website already selected. You can see the Desktop story here: https://dev-3.babel.hathitrust.org/common/firebird/dist/storybook/?path=/story/search-bar--desktop&globals=viewport:bsLg

In the Type Then Change Fields story, in the "add on panel" (storybook's sidebar toolbar thing), you can open the Interactions tab and press the ⏪ (Go to start) button to see the starting state and walk through each interaction.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I missed that, thanks!

@eumalin eumalin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes look good to me!

@carylwyatt carylwyatt merged commit c334bd3 into main Jun 11, 2026
6 checks passed
@carylwyatt carylwyatt deleted the ETT-1529-searchbar-bug branch June 11, 2026 15:37
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.

2 participants