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

AddOns: new design for the storage add-ons card #99433

Merged
merged 1 commit into from
Feb 20, 2025

Conversation

aneeshd16
Copy link
Contributor

@aneeshd16 aneeshd16 commented Feb 6, 2025

Fixes https://github.com/Automattic/martech/issues/3677
Fixes #95416

Proposed Changes

  • Updates the design for the storage add-on on the AddOns page. All storage add-on cards are replaced with a single storage card with a dropdown to allow users to select the desired storage. The dropdown values represent additional storage to be added.
  • Also fixes the issue described in Plans Grid: add-on pricing is recalculated unnecessarily dozens of times #95416, by memoizing some arrays. More details are in the code comments.

Why are these changes being made?

  • p58i-jlJ-p2

Testing Instructions

  • Go to /add-ons/<site slug>.
  • Confirm that a single storage add-ons card is present and it matches the design (3De4YM0tRV3Qs691ui6QAW-fi-7996_5289)
image
  • Opening the dropdown should show add-on values up to a maximum of 350GB.
image
  • Select any add-on from the list. Note the price mentioned - this should be the same as the price shown on the checkout page. (Monthly price * 12). Click on the "Buy add-on" button.

  • In the following examples, the 200GB add-on has been purchased.

  • Go back to /add-ons/<site slug>.

  • Confirm that the space used is 203GB. (200GB + 53GB platform default) (The site is on a Business plan)

  • Confirm that the dropdown values only contain add-ons till 150GB. (Since the maximum storage possible is 400GB).

  • image
  • The dropdown and CTA should not be shown if the maximum storage has been purchased.

image

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 UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • 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)?

@aneeshd16 aneeshd16 requested a review from a team February 6, 2025 21:02
@aneeshd16 aneeshd16 self-assigned this Feb 6, 2025
@aneeshd16 aneeshd16 requested a review from jeyip February 6, 2025 21:02
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 6, 2025
@aneeshd16 aneeshd16 force-pushed the update/storage-add-ons-card branch from 414725b to 0fdc665 Compare February 6, 2025 21:03

export const getAddOnsList = (): AddOnMeta[] => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function created a new array on every call. I have moved the array outside the function's scope so that the same reference is passed as the return value.

Comment on lines +21 to +24
const productSlugs = useMemo(
() => addOnMetas.map( ( { productSlug } ) => productSlug ),
[ addOnMetas ]
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

.map creates a new array on every call. So the useMemo function below, which uses productSlugs as a dependency, could not memoize the return value since the reference changed on every render.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@aneeshd16 aneeshd16 force-pushed the update/storage-add-ons-card branch from 0fdc665 to 44de9c7 Compare February 6, 2025 21:10
@matticbot
Copy link
Contributor

matticbot commented Feb 6, 2025

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • notifications
  • odyssey-stats
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/storage-add-ons-card on your sandbox.

@matticbot
Copy link
Contributor

matticbot commented Feb 6, 2025

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

App Entrypoints (~256 bytes added 📈 [gzipped])

name                   parsed_size           gzip_size
entry-main                 +1290 B  (+0.1%)     +256 B  (+0.0%)
entry-subscriptions         +415 B  (+0.0%)     +113 B  (+0.0%)
entry-stepper               +415 B  (+0.0%)     +113 B  (+0.0%)
entry-login                 +415 B  (+0.0%)     +113 B  (+0.0%)
entry-domains-landing       +415 B  (+0.1%)     +113 B  (+0.1%)
entry-browsehappy           +415 B  (+0.2%)     +113 B  (+0.2%)

Common code that is always downloaded and parsed every time the app is loaded, no matter which route is used.

Sections (~10467 bytes added 📈 [gzipped])

name                  parsed_size            gzip_size
add-ons                  +94302 B  (+27.7%)   +31144 B  (+29.3%)
site-marketing             +196 B   (+0.0%)     +812 B   (+0.2%)
marketing                  +196 B   (+0.0%)    +1021 B   (+0.4%)
settings-newsletter        -138 B   (-0.0%)     -824 B   (-0.5%)
settings-security          +103 B   (+0.0%)      +40 B   (+0.0%)
settings-performance       +103 B   (+0.0%)      +43 B   (+0.0%)
settings                    +93 B   (+0.0%)    +1096 B   (+0.4%)
settings-discussion         +58 B   (+0.0%)     +219 B   (+0.2%)
people                      -45 B   (-0.0%)     +402 B   (+0.2%)
settings-jetpack            +27 B   (+0.0%)     +614 B   (+0.4%)
update-design-flow          +13 B   (+0.0%)      -12 B   (-0.0%)
site-overview               +13 B   (+0.0%)       -2 B   (-0.0%)
signup                      -13 B   (-0.0%)       -4 B   (-0.0%)
plugins                     +13 B   (+0.0%)      -12 B   (-0.0%)
plans                       +13 B   (+0.0%)       -6 B   (-0.0%)
link-in-bio-tld-flow        +13 B   (+0.0%)      -12 B   (-0.0%)
jetpack-app                 +13 B   (+0.0%)       -7 B   (-0.0%)
hosting                     +13 B   (+0.0%)     +240 B   (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~8 bytes removed 📉 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected        +26 B  (+0.0%)       -8 B  (-0.0%)
async-load-signup-steps-plans                          +26 B  (+0.0%)       -8 B  (-0.0%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

@aneeshd16 aneeshd16 force-pushed the update/storage-add-ons-card branch from 44de9c7 to 7eb2a1a Compare February 7, 2025 01:19
@aneeshd16 aneeshd16 force-pushed the update/storage-add-ons-card branch from 7eb2a1a to cf14f6e Compare February 7, 2025 01:21
@aneeshd16 aneeshd16 added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Feb 7, 2025
@a8ci18n
Copy link

a8ci18n commented Feb 7, 2025

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

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 @aneeshd16 for including a screenshot in the description! This is really helpful for our translators.

@a8ci18n
Copy link

a8ci18n commented Feb 9, 2025

Translation for this Pull Request has now been finished.

Copy link
Contributor

@oswian oswian left a comment

Choose a reason for hiding this comment

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

Tests well and the code changes LGTM.

@oswian
Copy link
Contributor

oswian commented Feb 10, 2025

I just noticed that the updated version no longer displays ✔️ Purchased. Without it, I think the status of this addon feels a little less clear. Particularly because the other addons display a similar 'status' message. Was this intentional? Also should there still be a 'Manage add-on' button as per the other addons, in case they want to change payment method or cancel?

PR Prod
CleanShot 2025-02-10 at 19 54 02@2x CleanShot 2025-02-10 at 19 54 05@2x

@aneeshd16 aneeshd16 merged commit 0e4edcc into trunk Feb 20, 2025
13 of 14 checks passed
@aneeshd16 aneeshd16 deleted the update/storage-add-ons-card branch February 20, 2025 22:26
@github-actions github-actions bot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plans Grid: add-on pricing is recalculated unnecessarily dozens of times
4 participants