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

Add woocommerce_widget_layered_nav_term_anchor_text filter #40734

Merged
merged 6 commits into from Mar 15, 2024

Conversation

moory-se
Copy link
Contributor

@moory-se moory-se commented Oct 12, 2023

Submission Review Guidelines:

Changes proposed in this Pull Request:

Allow developers to change the anchor text of active filters. Often, one would add the taxonomy (Brand: Adidas).

How to test the changes in this Pull Request:

  1. Add this code snippet to your test site. This will make it so you can still add legacy widgets that have a modern block equivalent.
use Automattic\WooCommerce\Blocks\{ BlockTypesController, Package };

add_action(
	'widgets_init',
	function() {
		remove_filter(
			'widget_types_to_hide_from_legacy_widget_block',
			array(
				Package::container()->get( BlockTypesController::class ),
				'hide_legacy_widgets_with_block_equivalent'
			)
		);
	}
);
  1. Switch to a theme that is not block enabled, like Storefront or Twenty Nineteen.
  2. Go to Appearance > Widgets. Add a Filter by Attribute widget with a given attribute.
  3. Add a Legacy Widget and select "Active Product Filters" from the dropdown.
  4. Add this second code snippet to your test site:
add_filter(
	'woocommerce_widget_layered_nav_term_anchor_text',
	function ( $name ) {
		return $name . ' (changed)';
	},
	10
);
  1. Go to the Shop page on the front end and filter on any attribute.
  2. See that " (changed)" is added to items in the Active Product Filters widget.

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Add a filter to allow modifying the attribute term name in the Active Product Filters widget.

Comment

Allow developers to change the anchor text of active filters. Often, one would add the taxonomy (`Brand: Adidas`).
@github-actions github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Oct 12, 2023
@woocommercebot woocommercebot requested review from a team and coreymckrill and removed request for a team October 12, 2023 18:26
@github-actions
Copy link
Contributor

Hi ,

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

Copy link
Collaborator

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

Hi @moory-se , thanks for the PR! I left some code suggestions that will make sure our code linter is happy and also to make a minor security improvement.

@coreymckrill coreymckrill added the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Feb 13, 2024
moory-se and others added 3 commits February 13, 2024 06:53
…av-filters.php

Co-authored-by: Corey McKrill <916023+coreymckrill@users.noreply.github.com>
…av-filters.php

Co-authored-by: Corey McKrill <916023+coreymckrill@users.noreply.github.com>
@coreymckrill
Copy link
Collaborator

@moory-se Could you please update the PR description to include step-by-step instructions on how to test this? Here are some pointers on testing instructions: https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

@moory-se
Copy link
Contributor Author

moory-se commented Feb 27, 2024

Hi,

I don't think this can be tested really? I added some testing instructions, but they are not very helpful I think. You might think different though, which would be great :)

@coreymckrill coreymckrill removed the needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. label Feb 29, 2024
@coreymckrill
Copy link
Collaborator

@moory-se Ah, I figured out why I was having trouble testing this. There is a block version of this widget now (which the filter you're adding won't affect) and so the old legacy version of the widget gets hidden. So I think if your site is already using the legacy version of the widget, you don't get automatically switched to the block version, but you can't add new instances of the old legacy version.

Have you tried the block version of the Active Filters widget? It looks like it actually adds a label telling you which attribute a term is for, which might solve your use case.

@moory-se
Copy link
Contributor Author

moory-se commented Mar 7, 2024

Hi,

The block version is not a viable option for us. We have ~850 filters that are conditionally shown on the site. Handling those numbers with the block version would be impractical.

This PR is for the widget version, which is not deprecated. The PR also has 0 breaking changes, adds lots of value with very little code. Hope to get this merged :)

@coreymckrill coreymckrill self-requested a review March 7, 2024 23:37
@coreymckrill
Copy link
Collaborator

Gonna close and re-open this to jumpstart the stuck tests.

Copy link
Collaborator

@coreymckrill coreymckrill left a comment

Choose a reason for hiding this comment

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

@moory-se I figured out how to make the old legacy widgets available for testing, and I updated your testing instructions accordingly. Filter works fine in testing 👍

@coreymckrill
Copy link
Collaborator

@moory-se

The block version is not a viable option for us. We have ~850 filters that are conditionally shown on the site. Handling those numbers with the block version would be impractical.

Could you explain a bit more about why the block version can't handle ~850 attributes, but the old legacy version can? I think this would be good feedback for our blocks teams.

@coreymckrill coreymckrill merged commit b13f0b0 into woocommerce:trunk Mar 15, 2024
31 of 33 checks passed
@github-actions github-actions bot added this to the 8.8.0 milestone Mar 15, 2024
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Mar 15, 2024
@moory-se
Copy link
Contributor Author

@moory-se

The block version is not a viable option for us. We have ~850 filters that are conditionally shown on the site. Handling those numbers with the block version would be impractical.

Could you explain a bit more about why the block version can't handle ~850 attributes, but the old legacy version can? I think this would be good feedback for our blocks teams.

Doing it "manually" - in the UI - is a nightmare. It takes ~50 seconds to load the page with all the blocks. I have not digged very deep to debug this (we simply installed Classic Widgets and get on with our lives), so I'm not really sure why. I think it's partially due to the way blocks are stored, and partially because of the rendering process is not optimized for these volumes. I would guess the latter is the major part.

To fix this, we actually add them by code. The the_widget() function is very simple to call - you pass in every "instance data" as an array. The blocks counterpart - do_blocks() - expects some HTML-ish with the data as JSON-ish. It's error prone and cumbersome to work with.

Also, adding custom fields (such as a tooltip) to any widget is very simple with the classic widgets.

We've actually developed a custom system (outside woocommerce/wordpress) to manage all the filters/layered navigation, and the data/configuration is exported to woocommerce for rendering. I think this is the only way to handle many filters in woocommerce, as opposed to many other platforms where they are configured on attribute ("use as filter") and/or category ("attributes to use for filtering on this category") level.

@coreymckrill
Copy link
Collaborator

Thanks! I'll pass this along.

@alopezari alopezari added needs: internal testing Indicates if the PR requires further testing conducted by Solaris status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs: internal testing Indicates if the PR requires further testing conducted by Solaris plugin: woocommerce Issues related to the WooCommerce Core plugin. status: analysis complete Indicates if a PR has been analysed by Solaris type: community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants