Added personalization to purchase extension task #4849

octaedro · 2020-07-22T12:57:06Z

Fixes #4578

This PR adds personalization to the Purchase extension task list item.

It should:

Display the extension name if the user selected one extension: Purchase and install [extension_name] extension
Apply the same for themes: Purchase and install [theme_name] theme
Keep the task description generic if the user selected more than one product

Accessibility

I've tested using only a keyboard (no mouse)
I've tested using a screen reader
All animations respect prefers-reduced-motion
All text has at least a 4.5 color contrast with its background

Screenshots

Detailed test instructions:

Extensions

Go to the 3rd step of the OBW (URL: /wp-admin/admin.php?page=wc-admin&path=%2Fprofiler&step=product-types).
Select only one paid extension. Be sure you don't have a paid theme selected too.
Go back to the Dashboard or Home screen (URL: /wp-admin/admin.php?page=wc-admin).
Now the item Purchase and install [extension_name] extension should be visible in the task list.

Themes

Now unselect the paid extension, choose a free one and go to the 5th step of the OBW and select a paid theme.
Go back to the Dashboard or Home screen (URL: /wp-admin/admin.php?page=wc-admin).
Now the item Purchase and install [theme_name] theme should be visible in the task list.

More than one item

Try adding a paid extension and a paid theme, or two paid extensions.
The message Purchase & install extensions should be visible.

Changelog Note:

Dev: Added personalization to purchase extension task

This commit adds personalization to purchase extension task

This commit removes an unnecessary naming fix

jeffstieler

This tests well - nicely done. There are some issues with the way strings are being built and translated that need to be addressed.

jeffstieler · 2020-07-29T17:43:59Z

client/task-list/tasks.js

 			key: 'purchase',
-			title: __( 'Purchase & install extensions', 'woocommerce-admin' ),
+			title: sprintf(
+				__( '%s', 'woocommerce-admin' ),


This is effectively an untranslatable string.

Nice catch! I corrected it in the commit 7e8426d

jeffstieler · 2020-07-29T17:45:17Z

client/task-list/tasks.js


+	const purchaseAndInstallText =
+		uniqueItemsList.length === 1
+			? `Purchase & install ${ uniqueItemsList[ 0 ].name } ${ uniqueItemsList[ 0 ].type }`


This needs to be translatable - e.g. __( 'Purchase & install %s theme', 'woocommerce-admin' ). We should probably restrict the dynamic part of the string to just the Extension or Theme name.

Thank you @jeffstieler, this is also corrected in the commit 7e8426d

jeffstieler · 2020-07-29T17:45:29Z

client/task-list/tasks.js

+	const purchaseAndInstallText =
+		uniqueItemsList.length === 1
+			? `Purchase & install ${ uniqueItemsList[ 0 ].name } ${ uniqueItemsList[ 0 ].type }`
+			: 'Purchase & install extensions';


This needs to be wrapped in an i18n call.

jeffstieler · 2020-07-29T18:16:17Z

client/dashboard/utils.js

+	);
+
+	const uniqueItemsList = [
+		...new Set( [


Interesting trick!

jeffstieler · 2020-07-29T18:22:43Z

client/dashboard/utils.js

+ * @param {Array} installedPlugins Installed plugins.
+ * @return {Array} Objects with grouped product names and types.
+ */
+export function getGroupedOnboardingProducts( profileItems, installedPlugins ) {


Total nitpick on naming: technically this isn't grouping the products, it's labeling/categorizing them.

Thank you for the suggestion, I changed it in the commit e160630

jeffstieler · 2020-07-29T18:25:05Z

client/dashboard/utils.js

+		if ( product.label ) {
+			cleanedProduct = { type: 'extension', name: product.label };
+		} else {
+			cleanedProduct = { type: 'theme', name: product.title };


I've left other comments about translation - if these type values still end up in strings to display (which I don't think they should) they need to be passed through the i18n functions as raw strings here.

The method 'getGroupedOnboardingProducts' was given a more accurate name.

octaedro · 2020-07-30T14:46:38Z

Thank you for your review @jeffstieler, I addressed the suggested changes. Could you give this PR another look?

jeffstieler · 2020-07-30T16:12:54Z

client/task-list/tasks.js

+				? __( 'theme', 'woocommerce-admin' )
+				: __( 'extension', 'woocommerce-admin' );
+		purchaseAndInstallText = sprintf(
+			__( 'Purchase & install %s %s', 'woocommerce-admin' ),


If we're leaving two placeholders in the string they need to be positional - either a named replacement using interpolation or numerically indexed. Also we'd need to leave a note for the translators.

I'd personally prefer an alternative - this could turn into 2 translatable strings using the itemType logic above. Something like:

const purchaseAndInstallFormat = 'theme' === itemType ? __( 'Purchase & install %s theme, 'woocommerce-admin' ) : __( 'Purchase & install %s extension, 'woocommerce-admin' );

Thank you for your suggestion @jeffstieler. The modification was added in the commit c6e734d

jeffstieler

Code looks good and tests well. Nice work!

octaedro added status: in progress focus: onboarding labels Jul 22, 2020

octaedro self-assigned this Jul 22, 2020

Added personalization to purchase extension task

a163ac5

This commit adds personalization to purchase extension task

octaedro force-pushed the add/4578 branch from e83d9d1 to a163ac5 Compare July 22, 2020 19:18

octaedro added 2 commits July 22, 2020 16:28

Removed unnecessary naming fix

0a4984b

This commit removes an unnecessary naming fix

Corrected param description

a05f4ff

octaedro added [Status] Needs Review and removed status: in progress labels Jul 22, 2020

octaedro requested a review from a team July 22, 2020 20:17

jeffstieler suggested changes Jul 29, 2020

View reviewed changes

jeffstieler added needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. and removed [Status] Needs Review labels Jul 29, 2020

octaedro added 2 commits July 30, 2020 10:59

Solved translation problem

7e8426d

Naming corrected

e160630

The method 'getGroupedOnboardingProducts' was given a more accurate name.

octaedro added [Status] Needs Review and removed needs: author feedback The issue/PR needs a response from any of the parties involved in the issue. labels Jul 30, 2020

jeffstieler reviewed Jul 30, 2020

View reviewed changes

Modified string substitution

c6e734d

jeffstieler approved these changes Aug 3, 2020

View reviewed changes

jeffstieler added [Status] Ready to Merge and removed [Status] Needs Review labels Aug 3, 2020

octaedro merged commit 7a68f82 into main Aug 3, 2020

octaedro deleted the add/4578 branch August 3, 2020 19:24

Added personalization to purchase extension task #4849

Added personalization to purchase extension task #4849

Uh oh!

Conversation

octaedro commented Jul 22, 2020

Accessibility

Screenshots

Detailed test instructions:

Extensions

Themes

More than one item

Changelog Note:

Uh oh!

jeffstieler left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

octaedro commented Jul 30, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeffstieler left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants