Fix/37599 Add Product Name in the `aria-label` attribute #37830

faisal-alvi · 2023-04-19T10:55:47Z

Submission Review Guidelines:

I have followed the WooCommerce Contributing Guidelines and the WordPress Coding Standards.
I have checked to ensure there aren't other open Pull Requests for the same update/change.
I have reviewed my code for security best practices.
Following the above guidelines will result in quick merges and clear and detailed feedback when appropriate.

Changes proposed in this Pull Request:

Adds a Product Name in the aria-label attribute on the cart page.

Closes #37599.

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

Use the storefront theme (this is to see the mini cart popup in the header).
Add a product to the cart.
Visit the cart page.
Activate the screen reader on your device. For Windows users, you can use NVDA or JAWS screen reader. For Mac users, you can use VoiceOver.
Once the screen reader is activated, navigate to the 'X' icon beside the product in the cart. Use the screen reader's keyboard commands to reach it. For example, use the "Tab" key to move between links, buttons, and form fields.
It should say "Remove XX from cart". Where XX is the product name.

Now visit the shop page, and check the top right side cart icon, hover over it and navigate to the 'X' icon in the popup.
Again, it should say "Remove XX from cart". Where XX is the product name.

qasumitbagthariya · 2023-04-19T11:30:40Z

QA Update ✅

The aria label should have 'Remove XX'; where XX is the product name. Earlier it was 'Remove this item'.

Testing Environment

WordPress: 6.2
Theme: Storefront 4.2.0
Woocommerce - 7.6.0
PHP: 8.0.21
Web Server: Nginx 1.20.2
Browser: Chrome
OS: Windows 11
Git Branch: faisal-alvi:fix/37599

QA Status- Pass ✅

Aljullu · 2023-04-19T11:55:15Z

plugins/woocommerce/templates/cart/cart.php

 										'<a href="%s" class="remove" aria-label="%s" data-product_id="%s" data-product_sku="%s">&times;</a>',
 										esc_url( wc_get_cart_remove_url( $cart_item_key ) ),
-										esc_html__( 'Remove this item', 'woocommerce' ),
+										sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),


Thanks for this PR, @faisal-alvi! I didn't do a full review, but leaving a question here (maybe for @opr?):

The Cart block uses Remove %s from cart. I don't have a strong preference between both strings, but it might be a good idea to unify them, so we reduce the work for translators. Reusing the existing one in the Cart block would also guarantee that we have available translations. What do you think?

I think this is a great idea, Remove %s from cart sounds good!

opr

Hey @faisal-alvi, thanks for working on this! I tested this and it works really, well, however I have requested some changes which you can see in-line.

I would also like to request changes to your testing notes in the main PR body.

Please could you add some steps to describe testing the mini cart, and could you also add some steps to describe testing this with a screen reader?

Thanks!

opr · 2023-04-19T11:32:06Z

plugins/woocommerce/templates/cart/cart.php

 										'<a href="%s" class="remove" aria-label="%s" data-product_id="%s" data-product_sku="%s">&times;</a>',
 										esc_url( wc_get_cart_remove_url( $cart_item_key ) ),
-										esc_html__( 'Remove this item', 'woocommerce' ),
+										sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),


We should add a translation hint above, to note that %s is the product name.

Like so: /* translators: %s is the product name */

opr · 2023-04-19T11:38:53Z

plugins/woocommerce/templates/cart/cart.php

 										'<a href="%s" class="remove" aria-label="%s" data-product_id="%s" data-product_sku="%s">&times;</a>',
 										esc_url( wc_get_cart_remove_url( $cart_item_key ) ),
-										esc_html__( 'Remove this item', 'woocommerce' ),
+										sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),


In this line, only the translated string "Remove %s" is escaped. Not the product name. The product name is filled in after the escape.

I think we should do it like this, to ensure the product name is escaped correctly, too:

Suggested change

sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),

esc_attr( sprintf( __( 'Remove %s', 'woocommerce' ), $product_name ) ),

opr · 2023-04-19T11:48:01Z

plugins/woocommerce/templates/cart/mini-cart.php

 							'<a href="%s" class="remove remove_from_cart_button" aria-label="%s" data-product_id="%s" data-cart_item_key="%s" data-product_sku="%s">&times;</a>',
 							esc_url( wc_get_cart_remove_url( $cart_item_key ) ),
-							esc_attr__( 'Remove this item', 'woocommerce' ),
+							sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),


Same as the suggestion above, we should do esc_attr on the final result.

Suggested change

sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),

esc_attr( sprintf( __( 'Remove %s', 'woocommerce' ), $product_name ) ),

We also need a /* translators: comment above this use of __

plugins/woocommerce/changelog/fix-37599

Co-authored-by: Thomas Roberts <[email protected]>

codecov · 2023-04-19T14:38:57Z

Codecov Report

Merging #37830 (e524e95) into trunk (e8fcfbb) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

❗ Current head e524e95 differs from pull request most recent head 78fce9a. Consider uploading reports for the commit 78fce9a to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             trunk   #37830     +/-   ##
==========================================
- Coverage     51.5%    51.5%   -0.0%     
- Complexity   17261    17292     +31     
==========================================
  Files          429      430      +1     
  Lines        79957    80078    +121     
==========================================
+ Hits         41215    41239     +24     
- Misses       38742    38839     +97

Impacted Files	Coverage Δ
...ocommerce/includes/admin/class-wc-admin-assets.php	`0.0% <ø> (ø)`
...eta-boxes/class-wc-meta-box-product-categories.php	`0.0% <0.0%> (ø)`
plugins/woocommerce/includes/class-wc-ajax.php	`4.3% <0.0%> (-0.1%)`	⬇️

... and 11 files with indirect coverage changes

faisal-alvi · 2023-04-20T09:04:51Z

@opr thanks for the feedback. The suggestions/changes are implemented. I've also added steps to describe testing the mini cart, and steps to describe testing this with a screen reader in the PR description: #37830 (comment).

rrennick

@faisal-alvi Thanks for submitting this. In addition to my comment below, the template version in both templates needs to be updated to 7.8.0.

rrennick · 2023-04-20T18:55:58Z

plugins/woocommerce/templates/cart/cart.php

-				$product_id = apply_filters( 'woocommerce_cart_item_product_id', $cart_item['product_id'], $cart_item, $cart_item_key );
+				$_product     = apply_filters( 'woocommerce_cart_item_product', $cart_item['data'], $cart_item, $cart_item_key );
+				$product_id   = apply_filters( 'woocommerce_cart_item_product_id', $cart_item['product_id'], $cart_item, $cart_item_key );
+				$product_name = apply_filters( 'woocommerce_cart_item_name', $_product->get_name(), $cart_item, $cart_item_key );


There are some $_product->get_name() calls further down in the template that should be updated to $product_name.

Correct, those are replaced too. Thanks 70cbbee

faisal-alvi · 2023-04-21T08:36:04Z

@rrennick Thanks for the feedback. The requested changes are implemented. Please check.

rrennick

@faisal-alvi Looks great. Thanks for participating in the contributor day.

opr

Looks great, thanks for your contribution @faisal-alvi!

SiKth · 2023-06-18T18:48:03Z

Hi,
This update might have caused and issue with the hook "woocommerce_cart_item_name". A lot of people use this to add information after the product name in the Cart and in the Checkout. Now all that data also gets output in the aria-label field. I have this on my site and when I search for this a lot of people recommend this solution to do just this. Anything we can look into?

Try this code for example, where the data comes from custom fields.

`add_filter( 'woocommerce_cart_item_name', 'cfwc_cart_item_name', 10, 3 );
function cfwc_cart_item_name( $name, $cart_item, $cart_item_key ) {
	
	if( isset( $cart_item['size'] ) ) {
		$name .= '<br><b>Size:</b> ' . $cart_item['size'] . '';
	}
	if( isset( $cart_item['color'] ) ) {
		$name .= '<br><b>Color:</b> ' . $cart_item['color'] . '';
	}
	
	return $name;
}`

…8999) * Send $product_name through woocommerce_cart_item_name filter only once. Closes #38744. * Replace additional product name filters, with already filtered variable. * add additional params to docblock of woocommerce_cart_item_name * bump template version to 7.9.0 * revert cart_item_name filter change in #37830 * Minor fixes * Bump template version --------- Co-authored-by: Ron Rennick <[email protected]> Co-authored-by: Jorge A. Torres <[email protected]>

* Send $product_name through woocommerce_cart_item_name filter once (#38999) * Send $product_name through woocommerce_cart_item_name filter only once. Closes #38744. * Replace additional product name filters, with already filtered variable. * add additional params to docblock of woocommerce_cart_item_name * bump template version to 7.9.0 * revert cart_item_name filter change in #37830 * Minor fixes * Bump template version --------- Co-authored-by: Ron Rennick <[email protected]> Co-authored-by: Jorge A. Torres <[email protected]> * Prep for cherry pick 38999 --------- Co-authored-by: Kathy <[email protected]> Co-authored-by: Ron Rennick <[email protected]> Co-authored-by: Jorge A. Torres <[email protected]> Co-authored-by: WooCommerce Bot <[email protected]>

faisal-alvi added 2 commits April 19, 2023 16:21

Fix/37599 Add Product Name in the aria-label

edb5453

adding changelog

4696721

github-actions bot added plugin: woocommerce Issues related to the WooCommerce Core plugin. type: community contribution labels Apr 19, 2023

woocommercebot requested review from a team and rrennick and removed request for a team April 19, 2023 10:56

Aljullu reviewed Apr 19, 2023

View reviewed changes

opr requested changes Apr 19, 2023

View reviewed changes

opr reviewed Apr 19, 2023

View reviewed changes

plugins/woocommerce/changelog/fix-37599 Outdated Show resolved Hide resolved

Update plugins/woocommerce/changelog/fix-37599

da0d28e

Co-authored-by: Thomas Roberts <[email protected]>

peterfabian added the Contributor Day - H1 2023 label Apr 19, 2023

faisal-alvi added 3 commits April 20, 2023 14:03

update text: Remove %s from cart

7d4a7ca

escape the $product_name

158748a

translation hint for the product name

bfb1f08

faisal-alvi requested a review from opr April 20, 2023 09:04

rrennick suggested changes Apr 20, 2023

View reviewed changes

faisal-alvi added 2 commits April 21, 2023 14:02

updating the product name var

70cbbee

templates version bump to 7.8.0

a4c88e9

faisal-alvi requested a review from rrennick April 21, 2023 08:36

rrennick previously approved these changes Apr 21, 2023

View reviewed changes

add filter PHPDocs

e524e95

rrennick dismissed their stale review via e524e95 April 21, 2023 13:52

more filter PHPDocs

78fce9a

rrennick approved these changes Apr 21, 2023

View reviewed changes

opr approved these changes Apr 24, 2023

View reviewed changes

rrennick merged commit 2460532 into woocommerce:trunk May 11, 2023

github-actions bot added this to the 7.8.0 milestone May 11, 2023

SiKth mentioned this pull request Jun 19, 2023

woocommerce_cart_item_name might break new function for aria-label #38795

Open

rrennick pushed a commit to helgatheviking/woocommerce that referenced this pull request Jun 30, 2023

revert cart_item_name filter change in woocommerce#37830

e527a4b

jorgeatorres mentioned this pull request Jul 3, 2023

Prevent possible duplication in woocommerce_cart_item_name filter #39067

Closed

11 tasks

	sprintf( esc_attr__( 'Remove %s', 'woocommerce' ), $product_name ),
	esc_attr( sprintf( __( 'Remove %s', 'woocommerce' ), $product_name ) ),

Fix/37599 Add Product Name in the aria-label attribute #37830

Fix/37599 Add Product Name in the aria-label attribute #37830

Uh oh!

Conversation

faisal-alvi commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Submission Review Guidelines:

Changes proposed in this Pull Request:

How to test the changes in this Pull Request:

Uh oh!

qasumitbagthariya commented Apr 19, 2023

QA Update ✅

Uh oh!

Aljullu Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

opr Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

opr left a comment

Choose a reason for hiding this comment

Uh oh!

opr Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

opr Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

opr Apr 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Apr 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

faisal-alvi commented Apr 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rrennick left a comment

Choose a reason for hiding this comment

Uh oh!

rrennick Apr 20, 2023

Choose a reason for hiding this comment

Uh oh!

faisal-alvi Apr 21, 2023

Choose a reason for hiding this comment

Uh oh!

faisal-alvi commented Apr 21, 2023

Uh oh!

rrennick left a comment

Choose a reason for hiding this comment

Uh oh!

opr left a comment

Choose a reason for hiding this comment

Uh oh!

SiKth commented Jun 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Fix/37599 Add Product Name in the `aria-label` attribute #37830

Fix/37599 Add Product Name in the `aria-label` attribute #37830

faisal-alvi commented Apr 19, 2023 •

edited

Loading

codecov bot commented Apr 19, 2023 •

edited

Loading

faisal-alvi commented Apr 20, 2023 •

edited

Loading

SiKth commented Jun 18, 2023 •

edited

Loading