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

Show Facet widgets on taxonomy archives #2837

Merged
merged 1 commit into from
Jun 20, 2022
Merged

Show Facet widgets on taxonomy archives #2837

merged 1 commit into from
Jun 20, 2022

Conversation

burhandodhy
Copy link
Contributor

Description of the Change

This PR adds the ability to show the Facet widgets on the taxonomy archives.

Closes #2808

Verification Process

  • Add the Facet Widget.
  • Go to a product category archive
  • See the facets

Changelog Entry

Fixed: Show Facet widgets on taxonomy archives

Credits

Props @burhandodhy

@burhandodhy burhandodhy added this to the 4.2.1 milestone Jun 9, 2022
@felipeelia felipeelia removed their assignment Jun 9, 2022
@felipeelia
Copy link
Member

@burhandodhy PHP linting found a problem in the code. Do you mind addressing it and then assigning the PR back to be for review? I think we might be being too simplistic in that new list of checks but I can check that later.

@burhandodhy
Copy link
Contributor Author

@felipeelia liniting issues are fixed. For the check, I replicate the same behaviour which we have before.

@felipeelia
Copy link
Member

felipeelia commented Jun 9, 2022

For the check, I replicate the same behaviour which we have before.

Are you sure that

		if ( ! ( ( function_exists( 'is_product_category' ) && is_product_category() )
			|| $query->is_post_type_archive()
			|| $query->is_search()
			|| ( is_home() && empty( $query->get( 'page_id' ) ) ) )
		) {

behaves the same way

if ( $query->is_admin() || $query->is_singular() || $query->is_date() ) {

does?

Going to an author archive, just as an example, the code seems to have different behaviors.

Also, it is not needed to check if $query->is_admin() because we have a if ( is_admin() ) { a couple of lines above that already.

@burhandodhy
Copy link
Contributor Author

@felipeelia thanks for pointing out the author archive page. I totally missed out that case. I updated the check.

|| ( is_home() && empty( $query->get( 'page_id' ) ) ) )
) {
return false;
|| $query->is_tax()
Copy link
Member

Choose a reason for hiding this comment

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

@burhandodhy let's try to pass get_facetable_taxonomies() here (and possibly also get rid of is_tag() and is_category())

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@felipeelia I tried, but is_tax doesn't work for the built-in taxonomy page. We don't have any other option other than manually check every page.

Whether the query is for an existing custom taxonomy archive page. True for custom taxonomy archive pages, false for built-in taxonomies (category and tag archives). source

@felipeelia felipeelia removed their assignment Jun 14, 2022
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.

BUG: Facets on taxonomy archives
2 participants