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

Include zero count active facets in facet options #79

Merged
merged 4 commits into from
Mar 27, 2024

Conversation

ChristopherChudzicki
Copy link
Contributor

@ChristopherChudzicki ChristopherChudzicki commented Mar 27, 2024

What are the relevant tickets?

For mitodl/mit-learn#675

Description (What does it do?)

  • In Search page cleanup mit-learn#675, I added code in MIT Open to append zero-count active facets to the facet options. This PR moves that to course-search-utils.
  • Rewrites the facet display tests using testing library instead of Enzyme

Screenshots (if appropriate):

Note the zero-count active checkbox

Screenshot 2024-03-27 at 11 08 28 AM

How can this be tested?

Using mitodl/mit-learn#675, activate a search filter then EITHER:

  • add a text query that makes the filter have zero results
  • OR switch to one of the resource tabs with zero results
    The active filter should still show in the facet options list.

Comment on lines -2 to +3
import { shallow } from "enzyme"
import { render, screen, within } from "@testing-library/react"
import user from "@testing-library/user-event"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The vast majority of the line changes in this PR are from removing enzyme in this test file, and adding @testing-library/react, @testing-library/user-event.

I was going to add more tests in this file, and I did not want to add enzyme tests since they are fragile / don't work with React 17, 18. (In previous PR, I had to alter the test file because I extracted AvailableFacets from FacetDisplay https://github.com/mitodl/course-search-utils/pull/78/files#diff-0b2ec90271aa9923bd1a4e5747946849aa084238d325cc529cda4848f503b8a3). So I rewrote the enzyme tests with @testing-library.

I also added some tests... E.g., I didn't see tests for clicking the expand/collapse buttons.

@abeglova abeglova self-assigned this Mar 27, 2024
Copy link
Contributor

@abeglova abeglova left a comment

Choose a reason for hiding this comment

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

👍

@ChristopherChudzicki ChristopherChudzicki merged commit 4962874 into main Mar 27, 2024
2 checks passed
@odlbot odlbot mentioned this pull request Mar 27, 2024
2 tasks
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