Update Pricing table design to current version #26253

gmjuhasz · 2022-09-16T08:45:45Z

Changes proposed in this Pull Request:

Changed design to match the current one here: TTzDk3dzct551hYWRXPFa3-fi-3908%3A48536
Added option to show discount percentage
Added option to add popover info to each item. This can be done by providing tooltipInfo and optoional tooltipTitle props to the PricingTabelItem component.
Switched up prices, now the current one is on the left.
Added Terms of Service note to the bottom of the table.
Now only the main column should have a white background, this can set by adding the primary prop to a PricingTableColumn.

Other information:

Have you written new tests for your changes, if applicable?
Have you checked the E2E test CI results, and verified that your changes do not break them?

Jetpack product discussion

1202844140176868-as-1202944779121698

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Fire up storybook with running npm run storybook:dev from the js-packages/storybook folder.
Play with the added options described in the proposed changes section
Check with Figma file if design matches.

matticbot · 2022-09-16T08:45:54Z

Caution: This PR has changes that must be merged to WordPress.com
Hello gmjuhasz! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer and confirm D87888-code works as expected before merging this PR. Once this PR is merged, please commit the changes to WP.com. Thank you!
This revision will be updated with each commit to this PR

github-actions · 2022-09-16T08:46:35Z

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run bin/jetpack-downloader test jetpack update/pricing-table-design to get started. More details: p9dueE-5Nn-p2

github-actions · 2022-09-16T08:46:59Z

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

✅ Include a description of your PR changes.
✅ All commits were linted before commit.
✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
✅ Add testing instructions.
✅ Specify whether this PR includes any changes to data or privacy.
✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.

Jetpack plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 27, 2022.

Boost plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 26, 2022.

Social plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 26, 2022.

Starter Plugin plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 26, 2022.

Protect plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 26, 2022.

Videopress plugin:

Next scheduled release: October 4, 2022.
Scheduled code freeze: September 26, 2022.

danielpost

Really nice work! I've only added a couple minor thoughts.

Two other things:

It seems like the non-functional tooltip icons are still present on the table items. I think these need to be replaced by working tooltips, and on mobile these tooltips need to be present on each item in a column.

This was a bug that was already present before your PR, but adding a price without a label causes the button to line up wrong. What do you think about fixing that in this PR as well?

projects/js-packages/components/components/icon-tooltip/index.tsx

projects/js-packages/components/components/pricing-table/index.tsx

projects/js-packages/components/components/product-price/index.tsx

gmjuhasz · 2022-09-20T08:14:50Z

Thanks @danielpost! I have a few questions about these:

It seems like the non-functional tooltip icons are still present on the table items. I think these need to be replaced by working tooltips

Sure, these can be changed as well! Should these be added by users, or should they have a default value? Should they always appear on desktop? I was just wondering if a straight-forward feature name don't need one, we might should not show anything there.

, and on mobile these tooltips need to be present on each item in a column.

Now my solution accepts one label for each table item, which is shown on large screens and on mobile as well. I guess if you want different popovers on mobile/desktop, or to have one on mobile but not on desktop, you can do that with using the tooltipInfo={ isLg ? null : 'Only mobile label' } /> approach. Do you think this is good enough, or do you have a better solution? If needed I can create another prop for mobile tooltips, which overwrites the default one on mobile, could be cleaner.

This was a bug that was already present before your PR, but adding a price without a label causes the button to line up wrong. What do you think about fixing that in this PR as well?

I'll look into this for sure, thanks for mentioning! 👍

danielpost · 2022-09-20T09:28:50Z

Should these be added by users, or should they have a default value?

Added by users IMO. They provide extra information about a specific row in the table, so I don't know if there's a default that would make sense.

Should they always appear on desktop?

Here's how I think it's supposed to work: Each row in the table can have a tooltip. On desktop, these appear next to the "title" of each row, and on mobile they appear next to each related item in a column (so if you have two columns, the same tooltip would appear in two different places).

Additionally, users can add the tooltip to an individual item (you already added this feature). I think on mobile these would override the tooltips for a row, but that's something we can double-check with Design.

Does that make sense?

- Fix bug with height with no leyend on price - Add popover for feature on mobile - These are overwritten if user gives specific info for an item - Replaced props for Popover component - made offPercentage to a promoLabel to be able to display other values

danielpost

Nice! Just a couple more thoughts. Did you look into the button positioning when the price doesn't have a label underneath it?

projects/js-packages/components/components/pricing-table/index.tsx

projects/js-packages/components/components/pricing-table/styles.module.scss

gmjuhasz · 2022-09-21T08:50:59Z

Did you look into the button positioning when the price doesn't have a label underneath it?

Yeah, the pseudo element here solves it!

oskosk · 2022-09-21T15:42:40Z

Pushed rebase in e218a8f and versions update in 66015b5

oskosk

LGTM! Merging. Please follow up with another PR if something looks off or any feedback was left unaddressed.

github-actions · 2022-09-21T16:24:17Z

Great news! One last step: head over to your WordPress.com diff, D87888-code, and deploy it.
Once you've done so, come back to this PR and add a comment with your SVN changeset ID (e.g. r12345-wpcom).

Thank you!

oskosk · 2022-09-21T16:33:15Z

r252976-wpcom

gmjuhasz added 3 commits September 15, 2022 16:07

Update design based on Figma file

db8a62b

changelog

ac2cd13

Fix popover inside p

23db7c8

matticbot added the Touches WP.com Files label Sep 16, 2022

github-actions bot added [JS Package] Components [Status] In Progress RNA labels Sep 16, 2022

Merge branch 'trunk' into update/pricing-table-design

37babdc

gmjuhasz marked this pull request as ready for review September 16, 2022 12:11

gmjuhasz and others added 2 commits September 16, 2022 14:12

Merge branch 'trunk' into update/pricing-table-design

c3551e4

Add ToS and @wordpress/elements package

b033249

gmjuhasz self-assigned this Sep 16, 2022

gmjuhasz requested a review from a team September 16, 2022 13:43

danielpost reviewed Sep 19, 2022

View reviewed changes

kangzj mentioned this pull request Sep 20, 2022

Search Plugin: Add plans view page #25915

Closed

gmjuhasz and others added 5 commits September 20, 2022 14:02

Address review

a03b4e1

- Fix bug with height with no leyend on price - Add popover for feature on mobile - These are overwritten if user gives specific info for an item - Replaced props for Popover component - made offPercentage to a promoLabel to be able to display other values

Merge branch 'trunk' into update/pricing-table-design

621b3ef

Fix tests

95a4bf5

Fix popover clip-path issue

57f60c8

Merge branch 'trunk' into update/pricing-table-design

765d379

danielpost reviewed Sep 20, 2022

View reviewed changes

gmjuhasz and others added 2 commits September 21, 2022 10:49

Address review#2

2d93e5f

Merge branch 'trunk' into update/pricing-table-design

5fdcac3

Merge branch 'trunk' into update/pricing-table-design

e071a97

oskosk requested a review from a team as a code owner September 21, 2022 15:42

oskosk added [Status] Needs Review This PR is ready for review. and removed [Status] In Progress labels Sep 21, 2022

oskosk approved these changes Sep 21, 2022

View reviewed changes

oskosk merged commit 31f4cf8 into trunk Sep 21, 2022

oskosk deleted the update/pricing-table-design branch September 21, 2022 16:21

github-actions bot removed the [Status] Needs Review This PR is ready for review. label Sep 21, 2022

dkmyta mentioned this pull request Sep 22, 2022

Improve PricingTable component items #26364

Merged

8 tasks

gmjuhasz mentioned this pull request Sep 26, 2022

Fix: Pricing table divider on small screens #26443

Merged

2 tasks

Update Pricing table design to current version #26253

Update Pricing table design to current version #26253

Uh oh!

Conversation

gmjuhasz commented Sep 16, 2022

Changes proposed in this Pull Request:

Other information:

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

Uh oh!

matticbot commented Sep 16, 2022

Uh oh!

github-actions bot commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielpost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmjuhasz commented Sep 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danielpost commented Sep 20, 2022

Uh oh!

danielpost left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmjuhasz commented Sep 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oskosk commented Sep 21, 2022

Uh oh!

oskosk left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Sep 21, 2022

Uh oh!

oskosk commented Sep 21, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

github-actions bot commented Sep 16, 2022 •

edited

Loading

github-actions bot commented Sep 16, 2022 •

edited

Loading

gmjuhasz commented Sep 20, 2022 •

edited

Loading

gmjuhasz commented Sep 21, 2022 •

edited

Loading