-
Notifications
You must be signed in to change notification settings - Fork 802
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
MU WPCOM: Port the wpcom block editor nux feature from the ETK #38446
Conversation
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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 appearing at the bottom of this PR are passing or skipped. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
fca89dc
to
f56f877
Compare
e42fc07
to
f48274d
Compare
9f541b9
to
170a25b
Compare
8c03ef4
to
955fd92
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and ensured the following works well on an Atomic site
- Welcome Tour ✅
- Blogging prompts modal ✅
- Purchase notice (Payments block) ✅
- Sharing modal ✅
Will test the following;
- Draft post modal & First post published modal
- Seller celebration modal -> Tested and worked well ✅
- Video celebration modal -> Tested and worked well ✅
Yes. It's buggy on production as the
Sorry for not being clear. I mean you have to open the Welcome Guide if it's not shown because they share the same toggle 😓 |
I'm not sure if I can approve this PR, but if it's unstable in production, I guess we can merge it? |
Yes. It will raise another PR to fix it! |
Sorry, I realized I was wrong. Referring to Automattic/wp-calypso#57019, the modal is shown only when you land on the post editor through the “Choose a design” since it's unclear for users why they land on the post editor after selecting a design. So, the correct steps are
|
Thanks for the update @arthur791004! Here's my testing result:
|
The modal does not seem to display even in production (i.e., ETK), too. 😶🌫️ |
hmmm...it's weird that I can see the modal by either ETK or jetpack-mu-wpcom 🤔 |
Am I missing to mention something 🤔 Screen.Recording.2024-07-30.at.4.50.46.PM.mov |
After opening the Welcome Modal and following the steps, I finally succeeded in testing the "Write your first post" modal! Thank you for your work covering all the scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe we're good to go 🙏 🚀
b850a79
to
117f76f
Compare
Related https://github.com/Automattic/dotcom-forge/issues/8241, Automattic/wp-calypso#92861
Proposed changes:
Tip
Changes are too large because I copied
wpcom-block-editor-nux
,dotcom-fse/lib
and@automattic/tour-kit
intojetpack-mu-wpcom
It would be better to follow the Test Instructions below to check whether the functionalities work as expected or not
@automattic/tour-kit
dependency as follows:@automattic/components
@automattic/tour-kit
@automattic/viewport-react
@automattic/components
to thejetpack-mu-wpcom
because:@automattic/calypso-analytics
package that uses theEventEmitter
from theevents
package. I'm unsure whether to introduce theevents
package on the browser side.i18n-calypso
package, and the issue is the same as above.@emotion/*
packages that are used by the following components and I don't think it makes sense to introduce the whole@emotion/*
packages for this purpose.LoadingPlaceholder
SiteThumbnail
UpsellMenuGroup
@automattic/tour-kit
is only used by thewpcom-block-editor-nux
, so I decided to move the whole package tojetpack-mu-wpcom
to get rid of those dependenciesjetpack-mu-wpcom
because I think they are not too complicatedPaginationControl
ClipboardButton
FormInputCheckbox
FormLabel
recordTracksEvent
callrecordTracksEvent
with thewpcomTrackEvent
as we did for other migrations@automattic/onboarding
starter-page-templates/store
wpcom-block-editor-nav-sidebar/src/store
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions:
Before getting started, you have to
Welcome Tour
Blogging prompts modal
Purchase notice (Payments block)
Draft post modal & First post published modal - Only for the "write" intent
Seller celebration modal - Only for the "sell" intent
Sell online
Sharing modal - Not for the
start-writing
ordesign-first
intentVideo celebration modal - Only for the "videopress" intent
/sites/:site
endpoint to get the value ofoptions?.launchpad_checklist_tasks_statuses?.video_uploaded
from the response to check whether the video has been uploaded