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

Task: Create Plans page for Jetpack App Site Creation #82135

Merged

Conversation

staskus
Copy link
Contributor

@staskus staskus commented Sep 25, 2023

Fixes: #82131

Investigation: wordpress-mobile/WordPress-iOS#21598 (comment)
Discussion: p1695414275007579-slack-CFFF01Q4V
More context: pcdRpT-3RO-p2,

Requirements

  • Free, Personal, Premium, Business, E-commerce Plans
  • Input: A domain name
  • Output: Selected plan

Description

Created Plans page for Jetpack App Site Creation. It is intended to be reused and integrated into the native app flows.

Added routes:

For mobile-app user agent:

  • jetpack-app redirects to jetpack-app/plans
  • jetpack-app/plans

Other cases:

  • jetpack-app redirects home
  • jetpack-app/plans redirects home

Tasks

Basic setup

  • Created jetpack-app-plans directory
  • Added an entry into sections.js with a path and module
  • Created controller.jsx that displays JetpackAppPlans component and passes query parameters
  • Created index.js defining a page and including jetpackAppPlans controller
  • Created main.jsx that shows a loading indicator while querying plans, and PlansFeaturesMain component once plans are loaded
  • Created style.scss with jetpack-app-plans style that includes plan-features-layout-switcher to support media queries when displaying plans grid

Functionality

  • Setup specific plan selection for the page. Pass intent props to PlansFeatureMain. Consider creating a similar intent to "plans-jetpack-app" but for a site creation, which includes free plan as well
  • Setup required query parameter for the feature in controller.jsxthat would satisfy the requirements
  • Handle plan selection (onUpgradeClick) to pass cartItem for a mobile application once the plan is selected
  • Consider handling removePaidDomain which is triggered when paid domain is set and free plan is selected
    ...

Style

  • Hide action for "yearly/monthly" interval switch
  • Hide site switcher that is displayed on iPad-size page
  • Finish setting up style.scss which would include all the relevant styles so the page would appear fine for phones, and tablets. See styles of other features that are using PlansFeaturesMain
  • Show "Choose the perfect plan" title, the same one shown in post-signup plans flow

Coding Standards

Testing Instructions

UI

  1. Open .com/jetpack-app-plans
  2. Confirm Plans selection page is opened
  3. Confirm Free, Personal, Premium, Business, and E-Commerce Plans are displayed
  4. Confirm there're no headers and sidebars
  5. Confirm the page is responsive to viewport and orientation changes

paid_domain_name parameter

  1. Pass paid_domain_name parameter (.com/jetpack-app/plans?paid_domain_name=greatdomain.com)
  2. Confirm tapping on Free opens a pop-up
  3. Confirm domain name is visible in the header text as well as next to the plans
  4. Confirm tapping on a plan redirects to the same page with appended plan_id and plan_slug parameter. The app can use this redirection to come back to the native flow.

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • https://wpcalypso.wordpress.com/devdocs/docs/testing/index.md 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-ajp-p2)?

- Created jetpack-app-plans directory
- Added an entry into sections.js with a path and module
- Created controller.jsx that displays JetpackAppPlans component and passes query parameters
- Created index.js defining a page and including jetpackAppPlans controller
- Created main.jsx that shows a loading indicator while querying plans, and PlansFeaturesMain component once plans are loaded
- Created style.scss with jetpack-app-plans style that includes plan-features-layout-switcher to support media queries when displaying plans grid
@staskus staskus added [Status] In Progress Mobile Application WordPress.com App for mobile labels Sep 25, 2023
@github-actions
Copy link

github-actions bot commented Sep 25, 2023

@matticbot
Copy link
Contributor

matticbot commented Sep 25, 2023

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

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

name                   parsed_size           gzip_size
entry-stepper              +1345 B  (+0.1%)     +105 B  (+0.0%)
entry-main                 +1345 B  (+0.1%)     +108 B  (+0.0%)
entry-subscriptions          +90 B  (+0.0%)      +50 B  (+0.0%)
entry-login                  +90 B  (+0.0%)      +50 B  (+0.0%)
entry-domains-landing        +90 B  (+0.0%)      +50 B  (+0.0%)
entry-browsehappy            +90 B  (+0.1%)      +50 B  (+0.1%)

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

Sections (~16726 bytes added 📈 [gzipped])

name                  parsed_size           gzip_size
jetpack-app             +438515 B    (new)  +130414 B    (new)
update-design-flow          +78 B  (+0.0%)      +12 B  (+0.0%)
plugins                     +78 B  (+0.0%)      +12 B  (+0.0%)
plans                       +78 B  (+0.0%)      +12 B  (+0.0%)
link-in-bio-tld-flow        +78 B  (+0.0%)      +12 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 (~12 bytes added 📈 [gzipped])

name                                             parsed_size           gzip_size
async-load-signup-steps-plans-theme-preselected        +78 B  (+0.0%)      +12 B  (+0.0%)
async-load-signup-steps-plans                          +78 B  (+0.0%)      +12 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.

@staskus
Copy link
Contributor Author

staskus commented Sep 25, 2023

Changelog

I created a basic structure for /jetpack-app-plans page following documentation guide for creating a new section. I used other features that include PlanFeaturesMain component as an example.

separate.page.for.domains.mov

Now, we can:

  • Open /jetpack-app-plans that shows a basic plans grid
  • Pass domainName parameter (/jetpack-app-plans?domainName=example.com) to display the domain on the cards and display a popup when free domain is selected
  • Handle onUpgradeClick and onUpgradeClick events

I added tasks to the description. I think the most important next step is to make sure we can set the required inputs, and get the required outputs from and into our app to support the requirements. There will be more issues and caveats that you may find, please include them into tasks and/or mention in the changelog.

CC: @guarani @ravishanker

@guarani
Copy link
Contributor

guarani commented Sep 25, 2023

I created a basic structure for /jetpack-app-plans page following documentation guide for creating a new section. I used other features that include PlanFeaturesMain component as an example.

Good find! This is working great, thanks for making this happen @staskus! The code looks good and I only removed some parameters from PlansFeaturesMain that are marked deprecated and re-tested.

I haven't found the time today to add anything else to this PR yet.

- Allow redirect_to parameter to URL which is then used to callback with a selected plan
- Do not pass selected plan if free plan is selected
@staskus
Copy link
Contributor Author

staskus commented Sep 26, 2023

Changelog

✅ Configured plan selection to include free, personal, premium, business, and e-commerce (c5cfdc1
✅ Hid the yearly/monthly selector (377e11f)
✅ Handle onUpgradeClick and removePaidDomain events (3c896d6)

More details on handling onUpgradeClick and removePaidDomain events

The goal was to find out the way to pass information from the page into the app. I wanted to avoid calling platform-specific code and keep things as simple as possible.

My solution is to use window.location.href API that can be observed on web views and handled appropriately.

Example

  1. We pass redirect_to parameter wp.com/jetpack-app-plans?domain_name="somedomain.com"&redirect_to="jetpackapp://plans
  2. When a user taps on a personal plan, we call window.location.href="jetpackapp://plans?cart_item={product_slug: value_bundle, product_id: 1003} (encoded)
  3. Apps can handle it using usual redirection observing:
    override func webView(_ webView: WKWebView, decidePolicyFor navigationAction: WKNavigationAction, decisionHandler: @escaping (WKNavigationActionPolicy) -> Void) {
        let url = navigationAction.request.url
        /// Depends on what "redirect_to" parameter we pass
        if url?.scheme == "jetpackapp" {
            if let url = url, let components = URLComponents(url: url, resolvingAgainstBaseURL: true), let cartItem = components.queryItems?.first?.value {
                print(cartItem) // {"product_id":1009,"product_slug":"personal-bundle"}
            }
          decisionHandler(.cancel)
          return
        }
        decisionHandler(.allow)
    }
  1. Then we can use existing cart APIs to add both domain and product to a cart after the site is created (Plans in Site Creation: Create functionality to add domain with a plan to a cart wordpress-mobile/WordPress-iOS#21636)

Printing window.location.href into console video

redirection.mov

Using a dummy plan selection in the site creation flow that sets window.location.href value and adds it to cart video

Simulator.Screen.Recording.-.iPhone.15.-.2023-09-26.at.16.27.41.mp4

Issues

I couldn't identify a way to test these Calypso changes on the app, our authentication methods for web views don't work with local Calypso builds. Therefore I resorted to mocking the outputs from within the app with a dummy HTML page to confirm that it's a valid way to observe and get the selected plan.

CC: @guarani @ravishanker

Use getPlanCartItem to get plan
@guarani
Copy link
Contributor

guarani commented Sep 26, 2023

👋 There's an error in the console:

Cannot read properties of undefined (reading 'getProductId')
Untitled.mov

Based on your screen recordings above, there was no error, and I'm not sure why it wasn't happening before and is happening now. Please let me know if I'm missing something. onUpgradeClick returns an array of plan items, so it has no getProductId method. I used getPlanCartItem to get the plan item from the array (there should only be one), and get the product ID and slug from there.

@staskus
Copy link
Contributor Author

staskus commented Sep 27, 2023

onUpgradeClick returns an array of plan items, so it has no getProductId method. I used getPlanCartItem to get the plan item from the array (there should only be one), and get the product ID and slug from there.

Interesting. onUpgradeClick returned to me a MinimalRequestCartProduct which only contained product_slug. I used this slug to get a full product and then take productId out of it. I'll look further at what could be the issue 🤔

Update:

This commit merged 2 days ago changed onUpgradeClick from returning a single item to returning an array. 😄

image

I merged with trunk after I made a changelog update

image

Thank you for making the changes, @guarani !

@staskus
Copy link
Contributor Author

staskus commented Sep 27, 2023

Changelog

✅ Hid main header and sidebar
✅ Added page header with titles
✅ Other refactorings and improvements to style and localization

Plans.different.sizes.mov

@staskus
Copy link
Contributor Author

staskus commented Sep 27, 2023

@southp, could you take a look at this draft PR when you have time? Most importantly if the whole structure for creating a new page makes sense.

We decided to create a separate page for Jetpack App Plan Selection during Site Creation so we wouldn't need to make too many alternations to existing flows and components. Outside a header and loading, most of the functionality is contained within PlanFeaturesMain component.

I'm not sure what are the rules behind the placement and naming of new pages. I put it into .com/jetpack-app-plans but I could restructure or rename it in a preferred way.

Thanks!

@staskus staskus changed the title [Draft] Task: Create Plans page for Jetpack App Site Creation Task: Create Plans page for Jetpack App Site Creation Sep 27, 2023
@staskus staskus marked this pull request as ready for review September 27, 2023 16:00
@staskus
Copy link
Contributor Author

staskus commented Oct 10, 2023

Nice work!
I have some changes that you can work on, but I think we are very close :)

@jdc91, thank you for the review and valuable comments! 🙇

I addressed most of the comments. I explained what I couldn't make work next to the comments.

@staskus staskus requested a review from ddc22 October 10, 2023 21:17
client/jetpack-app/page-middleware/layout.tsx Outdated Show resolved Hide resolved
client/jetpack-app/page-middleware/layout.tsx Show resolved Hide resolved
client/jetpack-app/plans/index.ts Outdated Show resolved Hide resolved
client/jetpack-app/plans/controller.tsx Outdated Show resolved Hide resolved
client/jetpack-app/plans/controller.tsx Outdated Show resolved Hide resolved
client/jetpack-app/plans/style.scss Outdated Show resolved Hide resolved
client/sections.js Outdated Show resolved Hide resolved
client/sections.js Outdated Show resolved Hide resolved
client/jetpack-app/plans/style.scss Outdated Show resolved Hide resolved
client/jetpack-app/plans/main.tsx Outdated Show resolved Hide resolved
@staskus staskus requested review from enejb and tyxla October 12, 2023 08:28
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

This looks good and works well in my testing 👍 Nice work 🙌

I've left a few suggestions for follow-ups and a few notes to keep in mind.

Feel free to ship as-is and address the feedback in future PRs based on your project's prioritization.

🚀

client/jetpack-app/plans/main.tsx Show resolved Hide resolved
client/jetpack-app/plans/main.tsx Show resolved Hide resolved
client/jetpack-app/plans/style.scss Show resolved Hide resolved
client/sections.js Outdated Show resolved Hide resolved
client/jetpack-app/index.ts Show resolved Hide resolved
client/jetpack-app/index.ts Show resolved Hide resolved
);

// Default to /plans page until we have more pages
page( '/jetpack-app', '/jetpack-app/plans' );
Copy link
Member

Choose a reason for hiding this comment

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

Note that when this lands in production, to access the new /jetpack-app route it in staging/production you'll need to request the route to be pointed to Calypso.

Here's an example recent request for another route: pMz3w-ilk-p2

client/jetpack-app/plans/main.tsx Show resolved Hide resolved
@staskus
Copy link
Contributor Author

staskus commented Oct 12, 2023

I added README.md and noted other improvements in the following task #82934. We want to make more UI adjustments so other improvements will fit well as well.

@staskus staskus merged commit 3d91f1c into trunk Oct 12, 2023
@staskus staskus deleted the task/82131-create-plans-page-for-jetpack-app-site-creation branch October 12, 2023 11:46
@github-actions 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 Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile Application WordPress.com App for mobile
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task: Create Plans page for Jetpack App Site Creation
7 participants