Add the WPCOM Block Editor NUX #38511

Copons · 2019-12-18T16:03:11Z

Changes proposed in this Pull Request

⚠️ I've opened this for reviews, but I definitely need to double check the test steps to make sure it actually works as expected.

Add a custom WPCOM Block Editor NUX to the FSE plugins.
Override the Core NUX (Welcome Guide) to trigger WPCOM's instead.
If on WPCOM, the NUX is per-user instead of per-site. (See D36911-code)

Notes

Currently the WPCOM NUX has the exact same copy as the Core NUX.

Although, the WPCOM NUX is intentionally without images, so that it won't be confused with the Core NUX while testing.

Testing instructions

For the WPCOM instructions, please refer to D36911-code.

Open the block editor and make sure the WPCOM NUX show up.
It's the first time, so it's expected to show up even if we had previously dismissed the Core NUX already.
Dismiss the NUX.
Reload the editor: make sure the NUX does not show up.
Open the browser dev tools, and navigate to: Application -> Local Storage -> [site URL, e.g. localhost:8889]. Wipe the local storage for your site by clicking on the 🚫icon.

Reload the editor: make sure the NUX does not show up.
From the More menu, click on Welcome Guide. The NUX will show up again.
Without dismissing the NUX, reload the editor: make sure the NUX will show up again.

Fixes #38299 & #39320

matticbot · 2019-12-18T16:03:14Z

Test live: https://calypso.live/?branch=add/wpcom-block-editor-nux

matticbot · 2019-12-18T16:14:52Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

Addison-Stavlo

I tested given the current instructions, everything seems to be working as expected.

From what I can understand the endpoint and apitFetch are what is allowing this to be per user as opposed to per site?

Addison-Stavlo · 2019-12-18T20:23:40Z

apps/full-site-editing/full-site-editing-plugin/wpcom-block-editor-nux/src/disable-core-nux.js

+import { select, dispatch, subscribe } from '@wordpress/data';
+import '@wordpress/nux'; //ensure nux store loads
+
+dispatch( 'core/nux' ).disableTips();


Is this dispatch necessary given what is below in subscribe ?

I thought it wasn't necessary too, but it turns out it is! 😄

This ensures that the Core NUX doesn't open on editor load, even if it's supposed to.
Removing it would mean that, if the Core NUX tries to open on editor load, we instead open the WPCOM NUX regardless of the stored user preference.

Basically the reason for that subscribe is so that if the user manually triggers the Core NUX (AFAIK the only way is with the Welcome Guide item in the More menu), we incept, disable it and open the WPCOM NUX instead, by also setting the user attribute as "is_nux_enabled = true" in the DB.

On editor load, this is not necessarily what we want.
E.g.

User dismissed the WPCOM NUX on Site A. The cross-site user preference is now: is_nux_enabled = false.

User creates Site B.

On first editor load, the Core NUX tries to open (as we aren't disabling it beforehand anymore).

Our inception intervenes disabling the Core NUX, and force-enabling the WPCOM NUX instead, even though the user already dismissed it on Site A (and the user preference is is_nux_enabled = false).

Additionally, the inception also force-updates the user preference as is_nux_enabled = true (until the WPCOM NUX is dismissed again).

It's super hard to follow, I know!
Let me know if you need me to clear up this comment a little bit 😄

Addison-Stavlo · 2019-12-18T20:27:46Z

apps/full-site-editing/full-site-editing-plugin/wpcom-block-editor-nux/src/store.js

+import apiFetch from '@wordpress/api-fetch';
+import { plugins, registerStore, use } from '@wordpress/data';
+
+use( plugins.persistence, {} );


Im a little confused how this is working and what it is doing for us. It reads to me like we are inheriting persistence functionality onto an empty object. How does this effect what we end up registering with registerStore (which seems to be using persist: true anyways) ?

Well pointed out, @Addison-Stavlo!

I'm not super clear on how the persistence works, and the Guten readme got me confused.
I've tried removing this line, and it seems it's persisting as good as before.

Apparently, use( plugins.persistence, {} ); creates a new localStorage item, which in this case contained only the automattic/nux state.
But, the persist: true added the automattic/nux state to the default WP localStorage, so it was indeed plenty enough.

Copons

Improve the state persistence and use modern PHP array notation

Apparently the modern PHP array notation is killed by phpcbf on commit 😭

Addison-Stavlo · 2019-12-19T15:22:36Z

Although, the WPCOM NUX is intentionally without images, so that it won't be confused with the Core NUX while testing.

Is the intention to add the images back in before merging?

Copons · 2019-12-19T15:30:10Z

Is the intention to add the images back in before merging?

The intention of this PR is to provide a foundation for having a custom WPCOM NUX.
My plan is to ask for new images and copy, once this has got a technical approoval.

Addison-Stavlo · 2019-12-19T15:51:13Z

My plan is to ask for new images and copy, once this has got a technical approoval.

Gotcha! Ok.
I approve as far as I can tell on the technical standpoint (it may be good to have another set of eyes glance over the PHP parts, but from what I can tell it looks good), but I also assume that part of the "technical approval" at this point may be allowing some more time for discussion on the corresponding p2 post?

Either way, if this gets to the point where you are ready to merge and this is just being held up by not having the 'approved' flag here on github, feel free to ping me for it. I am just holding off now b/c I don't want to move it into the 'Ready to Merge' column before we have the images ready, discussion finished, etc.

Addison-Stavlo · 2020-02-03T20:55:41Z

Rebased+

It looks like Core changed a couple things, so there are still some updates to make for this.

Addison-Stavlo · 2020-02-04T00:04:53Z

I "think" this is working as intended again. 😄

vindl · 2020-02-14T02:07:02Z

This works as expected in testing, but if we land this the override will be live in production and we still don't show appropriate images for the Dotcom case. What do you think about commenting out the override init to unblock merging of this, and we can enable it once the images are ready?

vindl · 2020-02-14T02:07:59Z

apps/full-site-editing/full-site-editing-plugin/full-site-editing-plugin.php

+function load_wpcom_block_editor_nux() {
+	require_once __DIR__ . '/wpcom-block-editor-nux/class-wpcom-block-editor-nux.php';
+}
+add_action( 'plugins_loaded', __NAMESPACE__ . '\load_wpcom_block_editor_nux' );


Related to my other comment, we could temporarily comment this action until Dotcom side of things is ready.

Addison-Stavlo · 2020-02-14T19:24:41Z

@vindl - Yeah we could do that, and then would we go ahead with merging the diff now as well? (I guess we wouldn't NEED to, but it may be good to have that taken care of) We would also need to comment out some of the subscribers that block the core welcome guide from popping up in this PR, otherwise there would be no welcome guide available at all after it ships.

The other option is re-adding the core images to this and merge it as a replica of the core one for now. That may be more desirable as the 'site-wide' preference would go into effect and the core nux/welcome guide would stop popping up on every new site or clear browser state.

I guess the downside of the second idea is shipping the new nux will force the new guide to pop up on EVERYONE's account the first time they load up the editor after it launches, and that does provide an opportunity to show off whatever 'new' design we will have as opposed to a copy of core's current version.

So I guess it comes down to, would we rather:

Move the welcome guide to being an account wide setting sooner rather than later.

or

Hold off for the opportunity to 'show off' whatever new design by pairing it with the launch of the account-wide setting?

I guess since we haven't heard much noise about the core Welcome Guide being super annoying since it went live like this, then holding off for the second option might be best. Commenting things out and shipping this the way you mention goes along with that.

vindl · 2020-02-14T19:56:50Z

I guess since we haven't heard much noise about the core Welcome Guide being super annoying since it went live like this, then holding off for the second option might be best. Commenting things out and shipping this the way you mention goes along with that.

Since you also prefer this option let's go with it then. I'd try to land both the PR and the diff, but make sure that they don't cause changes in prod behavior now (I haven't checked if we need to comment something out on Dotcom side too but we'll likely have to). Then when we have the designs ready we can flip the switch. That way I think it will require less work to introduce this, instead of keeping this PR open for a long time and going through rebase hell. :)

Addison-Stavlo · 2020-02-17T15:25:52Z

@vindl - I commented out the loader and tested things out. Things seem to work well and I am thinking this is the only thing we need to comment out on either side (will write another note on the diff). But since all of the apply_filters and do_actions are nested behind the commented out loader, I am thinking shipping the diff without anything commented out makes sense and won't be effected.

Feel free to test!

simison · 2020-02-18T09:41:41Z

One reason to update to our own would be to just override the link to .org guide with https://en.support.wordpress.com/wordpress-editor/ — but I don't think that's necessarily big need.

simison · 2020-02-18T12:59:46Z

Just noting that once we enable this, we'll need to update E2E tests to close it.

vindl · 2020-02-18T15:44:57Z

@vindl - I commented out the loader and tested things out. Things seem to work well and I am thinking this is the only thing we need to comment out on either side (will write another note on the diff). But since all of the apply_filters and do_actions are nested behind the commented out loader, I am thinking shipping the diff without anything commented out makes sense and won't be effected.

@Addison-Stavlo I retested this and everything seems to work as expected on my end.

noahtallen · 2020-03-03T23:07:55Z

One reason to update to our own would be to just override the link to .org guide

I just made this change so that it references .com support now in #39846

we'll need to update E2E tests to close it.

However, I did not realize I needed this update because I didn't see any failing e2e tests.... 🤔

noahtallen · 2020-03-03T23:38:27Z

Even though all of the e2e tests are failing in #39853, it turns out that we already do dismiss the modal in e2e tests:

wp-calypso/test/e2e/lib/gutenberg/gutenberg-editor-component.js

Lines 433 to 443 in 8cb9bff

    
           	async dismissEditorWelcomeModal() { 
        
           		if ( 
        
           			await driverHelper.isElementPresent( this.driver, By.css( '.edit-post-welcome-guide' ) ) 
        
           		) { 
        
           			await driverHelper.clickWhenClickable( 
        
           				this.driver, 
        
           				By.css( '.edit-post-welcome-guide .components-modal__header .components-button' ) 
        
           			); 
        
           		} 
        
           	} 
        
           }

And these classnames are still correct with what we've just shipped.

noahtallen · 2020-03-03T23:55:14Z

Turns out the failures are just related to the gutenberg 7.6+ updates. Nothing amiss with the NUX :)

lancewillett · 2020-03-11T06:13:17Z

Hi team — is there a P2 thread to reference for this project? I'd like to chime in with my own personal experience (and please tell me if there's a better place).

With the core guide I find it very annoying to see the tutorial on every internal P2 site that I change over to the block editor (per user, per site).

How might we only show it once per user (regardless of the site where they are opting in)?

With a typical end user, this might be less of an issue, of course.

simison · 2020-03-11T07:27:20Z

How might we only show it once per user (regardless of the site where they are opting in)?

That would be a good improvement indeed. Apart from it, and this is not very urgent, we should just hide the welcome modal for internal sites methinks. It doesn't make sense there and is just on the way.

Logged at #40040

simison · 2020-03-11T07:31:15Z

is there a P2 thread to reference for this project?

Nope, we've tracked this here originally: #38299

It's not much of a project, more of maintenance task to adapt the core feature to .com infra. That kind of adaptations are always related to the process of just keeping Gutenberg plugin up-to-date on .com. Each Gutenberg version bump has its own P2, but this specific feature spans over several version upgrades.

noahtallen · 2020-03-12T00:25:37Z

@lancewillett let me know if you continue seeing the issue. The option should be persisted to the database as of yesterday, so you should no longer see the welcome modal on other sites if you dismiss it.

lancewillett · 2020-03-12T03:29:31Z

@noahtallen Cool, thank you! I'll keep an eye out for it happening.

Addison-Stavlo · 2020-03-12T16:19:32Z

@lancewillett - for a p2 about this specifically: pbAok1-qw-p2

This is currently running per user on simple sites, and should be extended to atomic as well with the next release of the FSE plugin.

Copons added the [Status] In Progress label Dec 18, 2019

Copons requested review from a team as code owners December 18, 2019 16:03

Copons self-assigned this Dec 18, 2019

Copons added [Type] Enhancement [Team] Cylon labels Dec 18, 2019

Copons added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Dec 18, 2019

Addison-Stavlo reviewed Dec 18, 2019

View reviewed changes

Copons commented Dec 19, 2019

View reviewed changes

simison assigned Addison-Stavlo and unassigned Copons Jan 30, 2020

simison added the [Status] Needs Rebase label Jan 30, 2020

Copons and others added 6 commits February 3, 2020 13:44

Add a new WPCOM NUX plugin

11851d5

Simplify the API handling

db5b1d4

Improve code readability

9c7ed6a

REST fixes

cfed048

Improve the state persistence and use modern PHP array notation

6327bc3

added welcomeGuide check

372675b

Addison-Stavlo force-pushed the add/wpcom-block-editor-nux branch from 5d278bc to 372675b Compare February 3, 2020 20:52

Addison-Stavlo added [Status] In Progress and removed [Status] Needs Rebase [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 3, 2020

added disable for welcomeGuide

7151b39

Addison-Stavlo added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Feb 4, 2020

Addison-Stavlo mentioned this pull request Feb 7, 2020

Make "Welcome Guide" / NUX appear per an Account-Wide Setting. #39320

Closed

vindl reviewed Feb 14, 2020

View reviewed changes

commented out nux loader

3f55e7f

Addison-Stavlo merged commit 6732750 into master Feb 18, 2020

Addison-Stavlo deleted the add/wpcom-block-editor-nux branch February 18, 2020 16:21

matticbot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 18, 2020

Addison-Stavlo mentioned this pull request Feb 25, 2020

Block Editor: Welcome guide cannot be dismissed with Do Not Track enabled. #39641

Closed

simison mentioned this pull request Mar 11, 2020

Gutenberg NUX welcome modal: hide per user, not per site-per-user #40040

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the WPCOM Block Editor NUX #38511

Add the WPCOM Block Editor NUX #38511

Copons commented Dec 18, 2019 •

edited by Addison-Stavlo

Loading

matticbot commented Dec 18, 2019

matticbot commented Dec 18, 2019

Addison-Stavlo left a comment •

edited

Loading

Addison-Stavlo Dec 18, 2019

Copons Dec 19, 2019

Addison-Stavlo Dec 18, 2019

Copons Dec 19, 2019

Copons left a comment

Addison-Stavlo commented Dec 19, 2019

Copons commented Dec 19, 2019

Addison-Stavlo commented Dec 19, 2019

Addison-Stavlo commented Feb 3, 2020

Addison-Stavlo commented Feb 4, 2020

vindl commented Feb 14, 2020

vindl Feb 14, 2020

Addison-Stavlo commented Feb 14, 2020

vindl commented Feb 14, 2020

Addison-Stavlo commented Feb 17, 2020

simison commented Feb 18, 2020

simison commented Feb 18, 2020

vindl commented Feb 18, 2020

noahtallen commented Mar 3, 2020

noahtallen commented Mar 3, 2020

noahtallen commented Mar 3, 2020

lancewillett commented Mar 11, 2020

simison commented Mar 11, 2020 •

edited

Loading

simison commented Mar 11, 2020 •

edited

Loading

noahtallen commented Mar 12, 2020

lancewillett commented Mar 12, 2020

Addison-Stavlo commented Mar 12, 2020 •

edited by lancewillett

Loading

Add the WPCOM Block Editor NUX #38511

Add the WPCOM Block Editor NUX #38511

Conversation

Copons commented Dec 18, 2019 • edited by Addison-Stavlo Loading

Changes proposed in this Pull Request

Notes

Testing instructions

matticbot commented Dec 18, 2019

matticbot commented Dec 18, 2019

Addison-Stavlo left a comment • edited Loading

Choose a reason for hiding this comment

Addison-Stavlo Dec 18, 2019

Choose a reason for hiding this comment

Copons Dec 19, 2019

Choose a reason for hiding this comment

Addison-Stavlo Dec 18, 2019

Choose a reason for hiding this comment

Copons Dec 19, 2019

Choose a reason for hiding this comment

Copons left a comment

Choose a reason for hiding this comment

Addison-Stavlo commented Dec 19, 2019

Copons commented Dec 19, 2019

Addison-Stavlo commented Dec 19, 2019

Addison-Stavlo commented Feb 3, 2020

Addison-Stavlo commented Feb 4, 2020

vindl commented Feb 14, 2020

vindl Feb 14, 2020

Choose a reason for hiding this comment

Addison-Stavlo commented Feb 14, 2020

vindl commented Feb 14, 2020

Addison-Stavlo commented Feb 17, 2020

simison commented Feb 18, 2020

simison commented Feb 18, 2020

vindl commented Feb 18, 2020

noahtallen commented Mar 3, 2020

noahtallen commented Mar 3, 2020

noahtallen commented Mar 3, 2020

lancewillett commented Mar 11, 2020

simison commented Mar 11, 2020 • edited Loading

simison commented Mar 11, 2020 • edited Loading

noahtallen commented Mar 12, 2020

lancewillett commented Mar 12, 2020

Addison-Stavlo commented Mar 12, 2020 • edited by lancewillett Loading

Copons commented Dec 18, 2019 •

edited by Addison-Stavlo

Loading

Addison-Stavlo left a comment •

edited

Loading

simison commented Mar 11, 2020 •

edited

Loading

simison commented Mar 11, 2020 •

edited

Loading

Addison-Stavlo commented Mar 12, 2020 •

edited by lancewillett

Loading