Skip to content

Conversation

@carylwyatt
Copy link
Member

@carylwyatt carylwyatt commented Nov 3, 2025

The problem

This took an embarrassing amount of time to untangle. The logic of this component is very circular, and I got stuck in a loop for a while. But! I think I got this right in the end.

The desired behavior for the search bar, regardless of location, is that the user can select to search either the "collection" or the "website" using the dropdown in the search bar. If "website" is selected, the dropdown to filter the "collection" search disappears. If "collection" is selected, then the filter dropdown should return.

On prod right now, if you're on the www site on any page other than the homepage, you can try to select "collection" from the dropdown, but you can't actually switch to "collection." See: https://www.hathitrust.org/accessibility Click the SEARCH button in the navbar, then try to select "collection" from the search bar dropdown. Can't do it!

The issue is that there was a long string of conditionals that resulted in the search bar never displaying "collection" or the collection filter dropdown menu on any page other than the homepage. This is because, on page load, we want to default to the search bar pre-selecting "website" as the search option. There is a click handler that is supposed to update the search bar when the dropdown has been clicked, but there is some circular logic that ultimately overrides that click handler logic.

The solution

I added a boolean to the click handler to signify whether the dropdown was clicked, then used that logic to tell the template what to update in the search bar. This allows the default/on-load search bar option to remain "website" but gives the conditional some new options in case someone has clicked the dropdown, essentially bailing out to a new template option if the "dropdownSelected" boolean is true.

I also did a little bit of cleanup. There were some red herring-type log messages in the console about "_searchtype" having a null value:

Uncaught TypeError: Cannot set properties of null (setting 'value')

This ultimately had nothing to do with the actual bug, but it was easy enough to clean up with some conditionals.

Testing

Staged on dev-3: https://dev-3.www.hathitrust.org/about

There are probably a variety of states you could/should test:

  1. On this non-homepage page (/about), switch from "website" search to "collection" search, do a search, and make sure it takes you to ls search results
  2. Back on /about, do a website search ('collection' or 'member' usually has some results), then on the results page (/search/[search-terms]), switch to collection search and back to website search
  3. For good measure, you could go to https://dev-3.catalog.hathitrust.org/Search/Home and make sure the search bar still works as expected outside of the website (this did not work this morning, but I just fixed it!)

@carylwyatt carylwyatt requested a review from liseli November 4, 2025 14:43
Copy link

@liseli liseli left a comment

Choose a reason for hiding this comment

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

I'll approve this PR. I've followed the 3 sets proposed by Caryl, and the changes are working fine.

Checking this PR, I saw an error that is probably not related to this change. It could be related to the storage migration. Anyway, I'll print it below to share it.

I searched on the website using the term report, and I saw the following error when clicking on the first item.

image

@carylwyatt
Copy link
Member Author

@liseli Thanks for the bug report! This error makes sense in the context of dev-3 because dev-3 doesn't have the ingest reports in the specified directory like the prod server does. The ingest reports page is working as expected on prod: https://www.hathitrust.org/member-libraries/resources-for-librarians/contributor-toolkit/ingest-reports/

@carylwyatt carylwyatt merged commit c237530 into main Nov 4, 2025
5 checks passed
@carylwyatt carylwyatt deleted the ETT-787-collection-search branch November 4, 2025 21:15
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.

3 participants