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

Sensei Tailored Flow #71406

Merged
merged 182 commits into from Jan 30, 2023
Merged

Sensei Tailored Flow #71406

merged 182 commits into from Jan 30, 2023

Conversation

dadish
Copy link
Contributor

@dadish dadish commented Dec 21, 2022

Proposed Changes

  • Adds a Sensei Tailored flow.

p6rkRX-4VM-p2

Testing Instructions

  • Go to calypso.localhost:3000/setup/sensei to kickoff the Sensei Tailored flow.
  • Confirm everything works as expected.

You can also test this out with the calypso live link here: https://container-ecstatic-khayyam.calypso.live/setup/sensei

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-ajp-p2)?

pgk and others added 30 commits October 12, 2022 09:18
* VideoPress site options step (WIP)

* Submit options step
* VideoPress Onboarding v2: Create account.

* Adjust the flow steps slightly and remove nonexistent ones.
* Submit options step

* VideoPress onboarding, domain selection step

* Choose a domain styling

* Add video, studio, productions, com as suggested tlds
* Submit options step

* VideoPress onboarding, domain selection step

* Choose a domain styling

* Add video, studio, productions, com as suggested tlds

* VideoPress site options step (WIP)

* Submit options step

* VideoPress onboarding: select a plan (WIP)

* Update how we store site title, description and domain by using onboarding store

* VideoPress onboarding: choose a plan step

* Add translation for choose a domain step

* Rebase
* VideoPress onboarding theme

* VideoPress powered logo

* Update flow progress and fix svg properties in videopress logo
* Update flow progress and fix svg properties in videopress logo

* Site options refined

* Populate existing site title and description in site options onboarding
* VideoPress onboarding theme

* VideoPress powered logo

* Update flow progress and fix svg properties in videopress logo

* Site options refined

* Populate existing site title and description in site options onboarding

* Allow passing the products list down to the domain search results

* Choose a domain styling WIP

* Choose a domain asides and styling

* Domain styling

* Domain step, style placeholders
* Adding intro view styles for new VideoPress flow. Refactors
 "Jetpack powered" to be more inclusive of different "powered by" products so that we can show the V
ideoPress logo there instead of Jetpack.

* New VP logo, aligned vertically

* More updates to new design iteration.

* Seeing if playsinline helps with mobile safari autoplay.

* Fix merge conflict for showing jetpack powered badge.

* No longer need z-index hack!

* Adjustments to the triangles

* Restore previous CSS classname.

* Restoring code, unintentional change.

* Move color styles out of body so they aren`t applied globally.

* Fixing CSS after bad merge.

* remove top margin to fix centering.

Co-authored-by: Eduardo Villuendas <eduardo.villuendas@automattic.com>
* VideoPress Onboarding V2: plan selection refined

* Mobile layout for choose a plan
…ompletion (#68162)

* clear onboarding site options on flow start and flow completion

* clear domain as well when starting or completing videopress flow
* Mobile layout for choose a plan

* Trying to do stuff with things, but it's not working, WIP

* One more step: got it to the checkout page.

* Cleanup

* Cleanup

* Less parameters = more fun

* Add domain to cart

Co-authored-by: Panos Kountanis <panosktn@gmail.com>
* Mobile layout for choose a plan

* Trying to do stuff with things, but it's not working, WIP

* One more step: got it to the checkout page.

* Cleanup

* Cleanup

* Less parameters = more fun

* Add domain to cart

* Steps proxy on useStepNavigation

* Upate validation rules

Co-authored-by: Panos Kountanis <panosktn@gmail.com>
* Mobile layout for choose a plan

* Trying to do stuff with things, but it's not working, WIP

* One more step: got it to the checkout page.

* Cleanup

* Cleanup

* Less parameters = more fun

* Add domain to cart

* Steps proxy on useStepNavigation

* Upate validation rules

* Disable form button if site title is not set

* Add button feedback for plan selection

* Update style.scss

Co-authored-by: Panos Kountanis <panosktn@gmail.com>
* Mobile layout for choose a plan

* Trying to do stuff with things, but it's not working, WIP

* Cleanup

* VP Onboarding: clear onboarding site options on flow start and flow completion (#68162)

* clear onboarding site options on flow start and flow completion

* clear domain as well when starting or completing videopress flow

* Upate validation rules

* Disable form button if site title is not set

* VideoPress processing step WIP

* Processing step

Co-authored-by: John Caruso <johncaruso@gmail.com>
* Adding new launchpad flow for VideoPress!

* Updating to i2 of design.

* Moving styles to videopress.scss so all styles live in one place and can use the same color vars.

* Set text-overflow: ellipsis for long domains.

* Correcting `completing-purchase` path to use camel case.

* Adding new launchpad flow for VideoPress!

* Updating to i2 of design.

* Moving styles to videopress.scss so all styles live in one place and can use the same color vars.

* Set text-overflow: ellipsis for long domains.

* Fix redirect URI from checkout page

* Add plan to cart in order to add extra data for videomaker activation

* Fixing launchpad settings for VideoPress flow.

* Update client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/task-helper.ts

Co-authored-by: John Caruso <johncaruso@gmail.com>

* Update client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/task-helper.ts

Co-authored-by: John Caruso <johncaruso@gmail.com>

* Update client/landing/stepper/declarative-flow/internals/steps-repository/launchpad/translations.tsx

Co-authored-by: John Caruso <johncaruso@gmail.com>

Co-authored-by: thedebian <thedebian@users.noreply.github.com>
Co-authored-by: John Caruso <johncaruso@gmail.com>
* New styles and nearly there with functionality. Need to rebase to test fully.

* Set the site_vertical_name option so the theme choice gets set properly.

* Clear the selected design when finishing or starting over the flow.

* Added responsive styles for small screen.

* Add min height for large screens, fixes continue button from jumping.

* User better import paths.

* Changes to design based on feedback.
@edanzer
Copy link
Contributor

edanzer commented Jan 26, 2023

I just ran through an extra test of this. For the most part, fluid and nice flow. I did notice a few things on the Launchpad screen. Sounds like you may be skipping this screen in the future, so you may or may not fix these. They could also easily be resolved in follow up PRs.

  • The Go to Admin link (bottom of sidebar) does not work. We actually recently ran into this when introducing some other flows. I had it myself. The fix (for now) is to reintroduce the deprecated goNext method you were asked above to remove. You can see my exact commit fixing it for another flow here: 9b0278e We need to update launchpad to not rely on that, be we can handle that in a separate PR, and will circular around to remove goNext in the sensei flow when done.
  • There are some small styling issues you would never notice if you weren't working on Launchpad. You need to add an extra class (your .sensei class) to ensure all styles apply. Again, you can see a commit to fix the same thing here: 0afac59
  • Clicking the Publish first course button should turn off the launchpad screen off, which should mean a) My Home doesn't not redirect to launchpad and b) if you try to load launchpad, it redirects back to My Home. For some reason (b) is not applying. Small issue because users could only get back to that screen by entering the url manually or via the back button. This is something we can also troubleshoot and resolve may be something with launchpad itself.

@gikaragia
Copy link
Contributor

Sounds like you may be skipping this screen in the future, so you may or may not fix these. They could also easily be resolved in follow up PRs.

Indeed for now we will not fix this screen as we intend to remove it in the future. As discussed I also moved SenseiStepContainer outside of packages in 25f5d95. I guess that the reasoning that we placed it there is that it closely relates to StepContainer however putting it inside components is fine too.

@agrullon95
Copy link
Contributor

The styling looks great !
I went through the flow and successfully landed on the launchpad. The launchpad still has some issues like the Go to admin link missing on mobile view port, not working on desktop view, and the site not launching after publishing my first course. Since the flow is behind a feature flag and the launchpad screen is going to be removed in a follow up PR, I dont think its a big deal.

Indeed for now we will not fix this screen as we intend to remove it in the future. As discussed I also moved SenseiStepContainer outside of packages in 25f5d95. I guess that the reasoning that we placed it there is that it closely relates to StepContainer however putting it inside components is fine too.

Moving the SenseiStepContainer to this location makes sense to me. Lets double check with @alshakero to make sure its makes sense to move it into the stepper directory.

Copy link
Contributor

@edanzer edanzer left a comment

Choose a reason for hiding this comment

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

Approving this! This is a BIG PR. I have focused on:

  • Confirming that testing works - I can go through the flow without issues as a user.
  • Confirming that no harm is done - ie, no unanticipated (negative) effects or regressions outside of the feature flag. The feature flag provides a measure of safety for this PR.
  • Confirm the areas of code I know well look good.

I'm comfortable with all that. There are couple caveats:

  • I didn't dive too deeply into the logic on specific new Sensei screens. Some have a lot of complex logic. I haven't gone through the logic on those screens in detail. There is confirmation others have.
  • There are few issues with the Launchpad screen, but that screen will be removed in a subsequent PR.
  • There are some failing tests. Authors checked with Calypso team and it sounds like those can be ignored in this case.

@gikaragia gikaragia merged commit ea2fc39 into trunk Jan 30, 2023
@gikaragia gikaragia deleted the add/sensei-onboarding branch January 30, 2023 16:34
@github-actions github-actions bot removed [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 30, 2023
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.

None yet