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

VerticalResults: Hide applied filters when displaying no results #690

Merged
merged 1 commit into from
Apr 20, 2020

Conversation

oshi97
Copy link
Contributor

@oshi97 oshi97 commented Apr 16, 2020

When displayAllResults is true, before a page would display
applied filters in the results header, like it does for normal
results.

This PR sets showAppliedFilters to false when displaying no results, which will hide any applied filters in the header (so nlp filters, static/facet filters, and sort options)

J=SPR-2003
TEST=manual
Checked that page was displaying applied filters for no results
Made change
checked that filters no longer display for no results
checked that filters still display for regular results

@nbramblett
Copy link
Contributor

Shouldn't this kind of behavior be configurable in the noResults object?

@oshi97
Copy link
Contributor Author

oshi97 commented Apr 16, 2020

Shouldn't this kind of behavior be configurable in the noResults object?

hmm idk asking pms

@oshi97
Copy link
Contributor Author

oshi97 commented Apr 16, 2020

Rose said to just do this for now, in the future we can use allResultsForVertical.appliedQueryFilters

@oshi97 oshi97 changed the base branch from v0.14.0 to v0.13.3 April 17, 2020 13:48
@oshi97 oshi97 changed the base branch from v0.13.3 to v0.14.0 April 17, 2020 13:50
@oshi97 oshi97 changed the base branch from v0.14.0 to v0.13.3 April 17, 2020 15:31
When displayAllResults is true, before a page would display
applied filters in the results header, like it does for normal
results.

It's still the case that only nlp filters are displayed.

J=SPR-2003
TEST=manual
Checked that page was displaying applied filters for no results
Made change
checked that filters no longer display for no results
checked that filters still display for regular results
@oshi97 oshi97 force-pushed the hide-filters-when-noresults branch from f838ebf to 76c15df Compare April 17, 2020 15:34
@oshi97
Copy link
Contributor Author

oshi97 commented Apr 17, 2020

changed base to 13.3

Copy link
Contributor

@nbramblett nbramblett left a comment

Choose a reason for hiding this comment

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

Seems fine to me, probably want someone else to approve too though

@tmeyer2115
Copy link
Collaborator

tmeyer2115 commented Apr 17, 2020

When displayAllResults is true, before a page would display
applied filters in the results header, like it does for normal
results.

It's still the case that only nlp filters are displayed.

J=SPR-2003
TEST=manual
Checked that page was displaying applied filters for no results
Made change
checked that filters no longer display for no results
checked that filters still display for regular results

@oshi97 are the NLP filters displayed in this no results case? If so, I don't think that's something we want shown either.

@oshi97
Copy link
Contributor Author

oshi97 commented Apr 17, 2020

When displayAllResults is true, before a page would display
applied filters in the results header, like it does for normal
results.
It's still the case that only nlp filters are displayed.
J=SPR-2003
TEST=manual
Checked that page was displaying applied filters for no results
Made change
checked that filters no longer display for no results
checked that filters still display for regular results

@oshi97 are the NLP filters displayed in this no results case? If so, I don't think that's something we want shown either.

Sorry I reread my commit message and it was really unclear, this commit stops NLP filters from being displayed as well.

@oshi97 oshi97 merged commit 3f86300 into v0.13.3 Apr 20, 2020
@oshi97 oshi97 deleted the hide-filters-when-noresults branch April 20, 2020 15:47
tmeyer2115 pushed a commit that referenced this pull request Apr 24, 2020
When displayAllResults is true, before a page would display
applied filters in the results header, like it does for normal
results.

It's still the case that only nlp filters are displayed.

J=SPR-2003
TEST=manual
Checked that page was displaying applied filters for no results
Made change
checked that filters no longer display for no results
checked that filters still display for regular results
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.

None yet

3 participants