FSE: Update NUX slides #41966

ramonjd · 2020-05-08T01:47:33Z

Changes proposed in this Pull Request

We're updating the copy and image based on the description in #41960

After unsuccessful attempts to render <NuxPage /> inside <Guide /> conditionally^, I went with a filtered collection approach.

All suggestions for improvement wanted. :)

Testing instructions

Easiest way I found is to check out this branch

Sandbox a test site

Comment out the following lines:

	/* if ( ! isWpcomNuxEnabled || isSPTOpen ) {
		return null;
	} */

Run cd apps/full-site-editing && yarn dev --sync

Test Gutenboarding and non-Gutenboarding versions by setting isGutenboarding to true/false

isGutenboarding === true

isGutenboarding === false

Fixes #41960

^ The result of trying something like { isGutenboarding && <NuxPage { ...props } /> }

matticbot · 2020-05-08T01:47:35Z

Test live: https://calypso.live/?branch=update/fse-nux-slides

matticbot · 2020-05-08T01:52:12Z

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.

roo2

Tested this and it works, displays the old three step help for non gutenboarding editor and the new one on gutenboarding. @joanrho it looks like all of the copy is up to date, it's still using the old blue colors and the new image with the circle instead of W logo already. It looks like this PR is good to go 🚀

@ramonjd the one thing I'm confused about is that the translations for the non-gutenboarding case seem to have changed? When I look at the site in spanish, they are not translated! But actually that's the same with the onboarding text before this PR too... 🤔

ramonjd · 2020-05-08T08:07:03Z

When I look at the site in spanish, they are not translated! But actually that's the same with the onboarding text before this PR too

Oh! Thanks for pointing that out. I'm going to double check. Probably a good argument for just replacing the NUX journey completely then with @joanrho's new designs/copy.

simison · 2020-05-08T08:41:27Z

Hmm, indeed, I don't find some of those older strings from translation tool at all. That's worrying!

Could it be they need textdomain? __( 'Example', 'full-site-editing' )?

👋 @Automattic/i18n

simison · 2020-05-08T08:47:39Z

Could it be they need textdomain?

Looks like that: https://github.com/Automattic/wp-calypso/search?q=full-site-editing&unscoped_q=full-site-editing

ramonjd · 2020-05-08T08:56:28Z

Could it be they need textdomain? __( 'Example', 'full-site-editing' )?

Oh! 🤦

I added the domain in the master branch code just to check, but didn't see any changes:

Funny that the buttons don't have the text domain, but they are translated

akirk · 2020-05-08T10:06:11Z

Inside Calypso we don't need the text domain / ignore it. When in Gutenberg, translations are loaded via inlined JS in the HTML (using wp_set_script_translations()), there the text domain is important since the translations are loaded into the text domain. I'm unsure which context we're talking about here.

simison · 2020-05-08T11:07:04Z

This code would run in wp-admin context in Gutenberg, so sounds like we will need it.

Additionally, this (FSE plugin) code gets sent to .org repo and shipped as a plugin to Atomic sites. I'm not entirely sure where the translation happen but I'm assuming it happens at .org repo?

ramonjd · 2020-05-11T01:21:50Z

I'm not entirely sure where the translation happen but I'm assuming it happens at .org repo?

As far as I can tell, the translations are all there, e.g.,

https://translate.wordpress.org/projects/wp-plugins/full-site-editing/dev/it/default/?filters%5Bterm%5D=Welcome+to+your+website&filters%5Bterm_scope%5D=scope_any&filters%5Bstatus%5D=current_or_waiting_or_fuzzy_or_untranslated&filters%5Buser_login%5D=&filter=Apply+Filters&sort%5Bby%5D=priority&sort%5Bhow%5D=asc

https://translate.wordpress.org/projects/wp-plugins/full-site-editing/dev/fr/default/?filters%5Bterm%5D=Insert+text%2C+photos%2C+forms%2C+Yelp+reviews%2C+testimonials%2C+maps%2C+and+more.+Rearrange+the+blocks+on+your+page+until+they%E2%80%99re+just+right&filters%5Bterm_scope%5D=scope_any&filters%5Bstatus%5D=current_or_waiting_or_fuzzy_or_untranslated&filters%5Buser_login%5D=&filter=Apply+Filters&sort%5Bby%5D=priority&sort%5Bhow%5D=asc

I'll add the domain arg to the translations.

ramonjd · 2020-05-11T01:28:37Z

I've also made the new copy the default as per the comments over at #41960 (comment)

We were originally hiding the privacy slide so I've kept that for now for non-Gutenboarding sites until further instruction

simison · 2020-05-11T06:00:34Z

We were originally hiding the privacy slide so I've kept that for now for non-Gutenboarding sites until further instruction

Yup, that's good. That's because users who have old published sites also can see the modal.

sirreal · 2020-05-11T11:38:05Z

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

-			/>
-			*/ }
+			{ getWpcomNuxPages( isGutenboarding ).map( ( nuxPage ) => (
+				<NuxPage { ...nuxPage } />


This doesn't complain about missing key?

We test on wp-admin where these warnings don't seem to show up.

Added heading as the key in c0af4f6

This doesn't complain about missing key?

Doh, forgot the key. Thanks for adding.

simison

Tests well on both Gutenboarding and non-gutenboarding sites.

there's little jump when image loads to<img> tag that I think if we decide to keep the modal, we should work out by inlining the SVG in JS.

"Preview your site" picture has different height and propotions than other images, which feels off. We can fix that in follow-up tho:

Updated the preview image with new one that replaces the W icon with a circle

…er them out using `isGutenboarding`. I'm doing it this way because `<Guide />` was counting children regardless of whether we were rendering them via `{ true ? </> : null }`. I'm not sure if this is a bug or that I missed something.

simison · 2020-05-11T13:16:20Z

Rebased to get build-fse-plugin / Build FSE plugin step pass.

ramonjd added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] Full Site Editing [Goal] New Onboarding previously called Gutenboarding labels May 8, 2020

ramonjd requested review from a team May 8, 2020 01:47

ramonjd self-assigned this May 8, 2020

roo2 self-assigned this May 8, 2020

roo2 approved these changes May 8, 2020

View reviewed changes

simison mentioned this pull request May 8, 2020

Add notice for block patterns new location. #41963

Merged

Addison-Stavlo mentioned this pull request May 11, 2020

Update FSE to version 1.4 #42013

Merged

3 tasks

sirreal reviewed May 11, 2020

View reviewed changes

simison approved these changes May 11, 2020

View reviewed changes

simison mentioned this pull request May 11, 2020

Fix/cleanup yarn lock #41847

Merged

ramonjd and others added 5 commits May 11, 2020 16:14

Added custom Gutenboarding copy and images based on new designs

a758099

Updated the preview image with new one that replaces the W icon with a circle

Making the new copy the default.

b594906

Added text domain value 'full-site-editing' for translations

98312f4

Add key to NuxPage

f057ac9

simison force-pushed the update/fse-nux-slides branch from c0af4f6 to f057ac9 Compare May 11, 2020 13:15

simison added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels May 11, 2020

simison merged commit 2bb8525 into master May 11, 2020

simison deleted the update/fse-nux-slides branch May 11, 2020 16:05

matticbot removed the [Status] Ready to Merge label May 11, 2020

ramonjd mentioned this pull request May 26, 2020

FSE NUX: fix the dimensions of the preview image #42675

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FSE: Update NUX slides #41966

FSE: Update NUX slides #41966

ramonjd commented May 8, 2020

matticbot commented May 8, 2020

matticbot commented May 8, 2020

roo2 left a comment

ramonjd commented May 8, 2020

simison commented May 8, 2020 •

edited

Loading

simison commented May 8, 2020

ramonjd commented May 8, 2020

akirk commented May 8, 2020

simison commented May 8, 2020

ramonjd commented May 11, 2020

ramonjd commented May 11, 2020

simison commented May 11, 2020

sirreal May 11, 2020

simison May 11, 2020

simison May 11, 2020

ramonjd May 11, 2020

simison left a comment

simison commented May 11, 2020

FSE: Update NUX slides #41966

FSE: Update NUX slides #41966

Conversation

ramonjd commented May 8, 2020

Changes proposed in this Pull Request

Testing instructions

isGutenboarding === true

isGutenboarding === false

matticbot commented May 8, 2020

matticbot commented May 8, 2020

roo2 left a comment

Choose a reason for hiding this comment

ramonjd commented May 8, 2020

simison commented May 8, 2020 • edited Loading

simison commented May 8, 2020

ramonjd commented May 8, 2020

akirk commented May 8, 2020

simison commented May 8, 2020

ramonjd commented May 11, 2020

ramonjd commented May 11, 2020

simison commented May 11, 2020

sirreal May 11, 2020

Choose a reason for hiding this comment

simison May 11, 2020

Choose a reason for hiding this comment

simison May 11, 2020

Choose a reason for hiding this comment

ramonjd May 11, 2020

Choose a reason for hiding this comment

simison left a comment

Choose a reason for hiding this comment

simison commented May 11, 2020

simison commented May 8, 2020 •

edited

Loading