Update conditional when rendering search tabs #277
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Pull Request Test Coverage Report for Build 19475376408Details
💛 - Coveralls |
** Why are these changes being introduced: Currently, if the initial DOM rendering is done in a condition where there are no search results, the UI doesn't not include tabs to change to a different set of results - which prevents the user from potentially finding results that were initially excluded. ** Relevant ticket(s): * https://mitlibraries.atlassian.net/browse/use-172 ** How does this address that need: This updates the conditional to look for the presence of a query term, rather than the presence of results. This allows us to continue to hide the tabs on initial rendering, prior to a search being done, but should mean that the tabs are shown in all conditions after a search. ** Document any side effects to this change: There shouldn't be - the USE application only has a single search field, so looking for params[:q] should be sufficient to detect a search. GeoData uses other fields, but it relies on different partials and doesn't use tabbed results anyway. Ultimately I could see a helper method for "is there a search?", which we get close to with the existing readable_param method, but I don't want to poke this bear until we have a better sense of what a general solution should look like.
matt-bernhardt
force-pushed
the
use-172-no-results
branch
from
661005f to
0fe51e9
Compare
matt-bernhardt
changed the title
JPrevost
approved these changes
Nov 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the initial DOM rendering checks for the presence of search results before building the links to search tabs - because we don't want to show tabs on the initial interface prior to the user performing a search.
While it makes sense to hide the tabs before the user does a search, this has a consequence that tabs are also not shown if the user's search generates no results - which isn't quite what we want, because it is possible that another tab may actually have results.
Relevant ticket
https://mitlibraries.atlassian.net/browse/use-172
How does this address that need
This updates the conditional used for the tabs, going from "are there results" to "is there a search in the querystring"? This causes the tabs to be rendered in all cases where they are needed.
Side effects
One of the instances of this codebase, GeoData, provides an advanced search form - so searches might be present there without the
qparameter being present. This distinction is saved by the fact that GeoData also doesn't provide tabbed results (and relies on a different view partial for its search form in the first place, so it would never get to this check).While this is okay for now, if the decision is made down the road to implement tabbed results in GeoData, we will need to make this check a bit more robust. I could imagine that our next step would be to define a helper method for "has the user done a search" that would check not only for
qbut also for the various other URL parameters associated with different types of searches.This change would also be required if the USE UI project ends up providing any sort of advanced search form, or introducing a condition wherein the user might have done a search while not using the
qparameter.Developer
Accessibility
New ENV
Approval beyond code review
Additional context needed to review
E.g., if the PR includes updated dependencies and/or data
migration, or how to confirm the feature is working.
Code Reviewer
Code
added technical debt.
Documentation
(not just this pull request message).
Testing