-
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 (2nd try) #38674
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
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. |
@taipeicoder I made a change to fix the image is broken because they are loaded from the CDN (s0.wp.com) in production by default. Could you take another look? Thank you! |
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.
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
UPDATED (2nd try)
@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
UPDATED (2nd try)
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