791: Campaign details page #87744

ernestoSk13 · 2024-02-22T01:00:05Z

Related to #

Proposed Changes

In this PR I'm working with the new campaign details UI. The UI should comply to the one shown in Figma.

Testing Instructions

Login to any blog that has campaigns on calypso and go to one of them
Verify the different UI elements and compare it to Figma
Try the same but with a Woo product (flag: is_running_in_woo_site).

NOTE: Some elements like the CTA Button or the Weekly total row, are not included in this PR. Also the Preview button is still unclear where should we place it

#64626

Screenshots

WPCom blog

Woo

Pre-merge Checklist

Has the general commit checklist been followed? (PCYsg-hS-p2)
Have you written new tests for your changes?
Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
Have you checked for TypeScript, React or other console errors?
Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

… add/woo-express-blaze-784

sbarbosa

Hello @ernestoSk13, I did another review/test and found some other details in the page (I added some comments for those).

In some medium screens, the header shows both of the buttons, but they are not aligned to the right, and have no gap between them

client/components/breadcrumb/index.tsx

sbarbosa · 2024-02-23T13:40:09Z

client/my-sites/promote-post-i2/components/campaign-item-details/index.tsx

 	const overallSpendingFormatted = `$${ formatCents( total_budget_used || 0 ) }`;
 	const deliveryEstimateFormatted = getCampaignEstimatedImpressions( display_delivery_estimate );
 	const campaignTitleFormatted = title || __( 'Untitled' );
 	const campaignCreatedFormatted = moment.utc( created_at ).format( 'MMMM DD, YYYY' );
-	const devicesListFormatted = devicesList ? `${ devicesList }` : __( 'All' );


This is currently a valid target option, we shouldn't remove it from the page. I think it's OK to put in above or under Languages.

Hi @ernestoSk13, with the last update we lose this targeting option. It is still a valid option, I think we need to include it in the final details.

client/my-sites/promote-post-i2/components/campaign-item-details/index.tsx

client/my-sites/promote-post-i2/utils/index.ts

client/my-sites/promote-post-i2/components/campaign-item-details/index.tsx

therocket-gr · 2024-02-26T14:01:14Z

some comments from me as well

not sure if this is ok and whether we should just have a breakpoint so that the labels should display in mobile one on top of the other (rather than just allowing the text to flow to different lines)

same as below.. not sure if this is ok .. looks like the text on the left in relation to the button is not aligned or something is off

ernestoSk13 · 2024-02-26T21:05:24Z

hi @sbarbosa @therocket-gr I've fixed this behavior and another minor UI issues :) thanks for your review

ernestoSk13 · 2024-02-27T23:56:02Z

hi @sbarbosa I've cleaned the scss and the index file. The mobile views should be working now too :)

sbarbosa

Nice work @ernestoSk13, the page adapts very well to the different screens now.
Great job!

I only find a comment related to the device target list. With the last changes, we lose that data, and still remains a valid targeting option in the Create campaign widget.

sbarbosa · 2024-02-28T14:03:35Z

client/my-sites/promote-post-i2/components/campaign-item-details/index.tsx

 	const overallSpendingFormatted = `$${ formatCents( total_budget_used || 0 ) }`;
 	const deliveryEstimateFormatted = getCampaignEstimatedImpressions( display_delivery_estimate );
 	const campaignTitleFormatted = title || __( 'Untitled' );
 	const campaignCreatedFormatted = moment.utc( created_at ).format( 'MMMM DD, YYYY' );
-	const devicesListFormatted = devicesList ? `${ devicesList }` : __( 'All' );


Hi @ernestoSk13, with the last update we lose this targeting option. It is still a valid option, I think we need to include it in the final details.

therocket-gr

everything seems ok to me now (i will approve even though i see a comment by @sbarbosa.. )
nice work

sbarbosa

Thanks for the changes! The PR looks good 👍

a8ci18n · 2024-02-28T17:22:43Z

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/11500566

Some locales (Hebrew, Japanese) have been temporarily machine-translated due to translator availability. All other translations are usually ready within a few days. Untranslated and machine-translated strings will be sent for translation next Monday and are expected to be completed by the following Friday.

Thank you @ernestoSk13 for including a screenshot in the description! This is really helpful for our translators.

a8ci18n · 2024-03-04T00:28:56Z

Translation for this Pull Request has now been finished.

Ernesto Sánchez Kuri added 25 commits January 25, 2024 17:45

Adding UI changes to the campaigns table items

0e4ab8a

Adding campaign filter dropdown for mobile devices

fb540d4

Adding conversion rate to campaigns table

5849712

Adding conversion rate to campaigns table

bc4173c

Adding conversion rate to campaigns table

cf6da8e

Fixing issue with segment control hover

45b3b87

Merge branch 'trunk' of https://github.com/Automattic/wp-calypso into…

0198517

… add/woo-express-blaze-784

Fixing github comments

19b38b7

Fixing min width for mobile dropdown views

1e4285a

Fixing Clicks column width

5d2d338

Improving sort function for campaigns

5d818a5

Fixed issue with undefined strings"

3aeaf66

Fixing issue with dropdown selection

836954a

Adding logic to handle search filters on campaigns list

33cbbf3

Fixing conversion values

5398198

Merge branch 'trunk' into add/woo-express-blaze-784

a802d38

Adding campaign details page updates

bc9735b

Changing sort parameters to comply to dsp backend

1d28ae1

Adding new row

3c19796

Merge branch 'add/woo-express-blaze-784' into add/blaze-woo-express-791

0137954

Adding woo logic

53ce401

Updating filters

faf4b48

Merge branch 'trunk' into add/blaze-woo-express-791

18a1b55

Adding new UI to campaign detail page

b713840

Adding new UI to campaign detail page

ff7515f

ernestoSk13 requested review from sbarbosa, therocket-gr and jjolmo February 22, 2024 01:00

matticbot added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 22, 2024

Ernesto Sánchez Kuri added 3 commits February 22, 2024 14:51

Fixing github comments

3b11c5c

Merge branch 'trunk' into add/blaze-woo-express-791

1a68180

Fixing mobile view

863f239

sbarbosa marked this pull request as ready for review February 23, 2024 13:21

sbarbosa reviewed Feb 23, 2024

View reviewed changes

sbarbosa added 3 commits February 23, 2024 14:24

Merge branch 'trunk' into add/blaze-woo-express-791

2c984d4

Fixes some of the calculated label/values

d3a3a84

Removes the estimated impressions sub label

b08fc1c

Ernesto Sánchez Kuri added 4 commits February 26, 2024 09:58

Merge branch 'trunk' into add/blaze-woo-express-791

caf1cbf

Fixing mobile version

31c4ae2

Fixing issue with UI issues

68f4fd6

Fixing issue with UI issues

ff6cde5

Ernesto Sánchez Kuri added 5 commits February 27, 2024 12:19

Fixing issue with UI issues

3c77cc4

Fixing issue with UI issues

08d9286

Fixing issue with UI issues

f8e26f8

Fixing issue with wrong alignment

b8fbdb1

Cleaning code to avoid redundancy

5a4117e

sbarbosa reviewed Feb 28, 2024

View reviewed changes

therocket-gr approved these changes Feb 28, 2024

View reviewed changes

Ernesto Sánchez Kuri added 2 commits February 28, 2024 09:30

Adding audience section again

7b970d2

Removing duplicated interests label in mobile

a96bd70

sbarbosa approved these changes Feb 28, 2024

View reviewed changes

ernestoSk13 merged commit 437fd01 into trunk Feb 28, 2024
11 checks passed

ernestoSk13 deleted the add/blaze-woo-express-791 branch February 28, 2024 16:18

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

791: Campaign details page #87744

791: Campaign details page #87744

ernestoSk13 commented Feb 22, 2024

sbarbosa left a comment

sbarbosa Feb 23, 2024

sbarbosa Feb 28, 2024

therocket-gr commented Feb 26, 2024

ernestoSk13 commented Feb 26, 2024

ernestoSk13 commented Feb 27, 2024

sbarbosa left a comment

sbarbosa Feb 28, 2024

therocket-gr left a comment

sbarbosa left a comment

a8ci18n commented Feb 28, 2024

a8ci18n commented Mar 4, 2024

791: Campaign details page #87744

791: Campaign details page #87744

Conversation

ernestoSk13 commented Feb 22, 2024

Proposed Changes

Testing Instructions

Screenshots

Pre-merge Checklist

sbarbosa left a comment

Choose a reason for hiding this comment

sbarbosa Feb 23, 2024

Choose a reason for hiding this comment

sbarbosa Feb 28, 2024

Choose a reason for hiding this comment

therocket-gr commented Feb 26, 2024

ernestoSk13 commented Feb 26, 2024

ernestoSk13 commented Feb 27, 2024

sbarbosa left a comment

Choose a reason for hiding this comment

sbarbosa Feb 28, 2024

Choose a reason for hiding this comment

therocket-gr left a comment

Choose a reason for hiding this comment

sbarbosa left a comment

Choose a reason for hiding this comment

a8ci18n commented Feb 28, 2024

a8ci18n commented Mar 4, 2024