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

Editor Toolkit: update slide copy in NUX #46674

Merged
merged 3 commits into from
Nov 6, 2020
Merged

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Oct 22, 2020

Changes proposed in this Pull Request

Updating copy to emphasis the hidden nature of new sites rather than the private by default aspect, which we'll be rolling back with coming soon v2 (by default new sites will be in coming soon mode, but public (not indexable)

We're also showing all steps now that the Launch button is persisting in p9Jlb4-1SN-p2 and #46817

Screen Shot 2020-10-23 at 11 19 17 am

Context p1603330128417000-slack-C9EJ7KSGH

Testing instructions

The quick and dirty way of checking the changes is to comment out the following lines in wpcom-nux.js

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

Run yarn dev --sync from apps/editing-toolkit

Create a new site via /new

Sandbox that site

Navigate through the NUX modal to the last panel

Check that the copy reflect the changes in this PR

Also check that we're dislaying all the Nux Pages we return in getWpcomNuxPages()

@matticbot
Copy link
Contributor

@ramonjd ramonjd added [Goal] New Onboarding previously called Gutenboarding Coming Soon labels Oct 22, 2020
@ramonjd ramonjd requested a review from a team October 22, 2020 09:12
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Oct 22, 2020
@ramonjd ramonjd assigned ramonjd and unassigned ramonjd Oct 22, 2020
@matticbot
Copy link
Contributor

Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com
Please ensure your changes work on WordPress.com before merging.

D51631-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2

@matticbot
Copy link
Contributor

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.

@autumnfjeld autumnfjeld self-assigned this Oct 27, 2020
Copy link
Contributor

@autumnfjeld autumnfjeld left a comment

Choose a reason for hiding this comment

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

Copy updates confirmed! 👍🏼

Screen Shot 2020-10-27 at 14 31 00

@ramonjd ramonjd added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Oct 27, 2020
@a8ci18n
Copy link

a8ci18n commented Oct 27, 2020

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/5050272

Thank you @ramonjd for including a screenshot in the description! This is really helpful for our translators.

@ramonjd ramonjd removed the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Oct 28, 2020
@ramonjd
Copy link
Member Author

ramonjd commented Oct 28, 2020

Requesting review again since the copy and slide count have changed 🙇

@@ -108,17 +106,15 @@ function getWpcomNuxPages( isGutenboarding ) {
alignBottom: true,
},
{
heading: __( 'Private until you’re ready', 'full-site-editing' ),
heading: __( 'Hidden until you’re ready', 'full-site-editing' ),
Copy link

Choose a reason for hiding this comment

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

⚠️ This change will be queued for retranslation. We'll display the English text until then.

@a8ci18n
Copy link

a8ci18n commented Oct 29, 2020

This Pull Request is now available for translation here: https://translate.wordpress.com/deliverables/5050272

Thank you @ramonjd for including a screenshot in the description! This is really helpful for our translators.

Copy link
Member

@p-jackson p-jackson left a comment

Choose a reason for hiding this comment

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

Click “Launch” in the toolbar to share it with the world.

Last I knew the button said "Complete setup" but maybe that's changed now?

I seem to be doing something wrong because for some reason I'm not seeing any gutenboarding button in the toolbar at all ...

Comment on lines -117 to -118
// @TODO: hide for sites that are already public
shouldHide: ! isGutenboarding,
Copy link
Member

Choose a reason for hiding this comment

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

Technically thie @TODO hasn't been addressed, but I don't know who's responsibility it is or even when this modal would appear for a public site so 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

My inclination is to let the reference to "hidden" slide for now. The main reason we're showing this one for all users I think stems from p9Jlb4-1SN-p2

The intention the button will now be displayed to users regardless of their path into WordPress and will be combined with the Focused launch flow work currently in development by Team Luna.

Copy link

@razvanpapadopol razvanpapadopol Nov 6, 2020

Choose a reason for hiding this comment

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

I'd leave the @T0D0 or create an issue for this because from what I understand it is referring to the launched/un-launched status of a website and not to the onboarding (site creation) flow. So here is an example where we probably don't want the slide to be displayed:

  1. User created a site and landed in My Home or Plans (after abandoning cart).
  2. User launched the site via Site setup checklist in My Home without opening the editor.
  3. User navigates to the editor and the Launch button in the toolbar is gone since the site is already launched.

@p-jackson
Copy link
Member

Yes just confirmed, the launch button only appears in the editor when I'm un-sandboxed or when I haven't run yarn dev --sync. If I use this branch for the ETK then the launch button isn't there!

Screenshot 2020-10-29 at 5 03 14 PM

Is the launch button broken on master? 🙀

@ramonjd
Copy link
Member Author

ramonjd commented Oct 29, 2020

Last I knew the button said "Complete setup" but maybe that's changed now?

Thanks for asking @p-jackson !

p9Jlb4-1SN-p2 says

Persistent ‘Launch’ button displayed in the editor toolbar for both new and existing users
Re-label the button to read ‘Complete setup’ ‘Launch’

Not sure either! cc @ollierozdarz

I seem to be doing something wrong because for some reason I'm not seeing any gutenboarding button in the toolbar at all ...

Maybe we're waiting on #46817? is that right @ciampo ?

@ollierozdarz
Copy link

Last I knew the button said "Complete setup" but maybe that's changed now?

Thanks for asking @p-jackson !

p9Jlb4-1SN-p2 says

Persistent ‘Launch’ button displayed in the editor toolbar for both new and existing users
Re-label the button to read ‘Complete setup’ ‘Launch’

Not sure either! cc @ollierozdarz

@ramonjd the button currently says 'Complete setup' for users who came through Gutenboarding, however we've planned to change the lablel to say 'Launch' with the new persistent launch button work.

@p-jackson
Copy link
Member

@ramonjd ok I've confirmed that the launch button works on the latest master, but was broken on the version of master that you started your PR on (205c3bd)

So you could rebase if you wanted to confirm, but yes it looks like the launch button is all good now.

@ramonjd
Copy link
Member Author

ramonjd commented Oct 29, 2020

@ramonjd ok I've confirmed that the launch button works on the latest master, but was broken on the version of master that you started your PR on (205c3bd)

Very grateful for your investigations!

ramonjd and others added 2 commits October 29, 2020 15:17
…he private by default aspect, which we'll be rolling back with coming soon v2 (by default new sites will be in coming soon mode, but public (not indexable)
Updating copy to remove reference to coming soon and highlight the launch button
Showing all steps now. No filtering of nux pages.
@ramonjd ramonjd force-pushed the update/gutenboarding-nux-copy branch from a3ec07e to 5f7dda4 Compare October 29, 2020 04:19
@ciampo
Copy link
Contributor

ciampo commented Oct 29, 2020

Maybe we're waiting on #46817? is that right @ciampo ?

Hey @ramonjd and @p-jackson! The gutenboarding/persistent-launch-button flag is only enabled in development at the moment. Without that feature flag, the "Launch" button is only shown for sites created with Gutenboarding.

For more context, you can refer to #46558 (introduction of the persistent launch button behind a flag, merged) and #46817 (enabling the feature flag for all environments, waiting to be merged)

(cc @razvanpapadopol )

@razvanpapadopol
Copy link

razvanpapadopol commented Oct 29, 2020

What @ciampo wrote above plus one note:

@ramonjd the button currently says 'Complete setup' for users who came through Gutenboarding, however we've planned to change the lablel to say 'Launch' with the new persistent launch button work.

We already replaced 'Complete setup' label with 'Launch' also for Gutenboarding in #46558 so at this moment we should just wait for strings to be translated and then merge this PR and #46817 🚢

@razvanpapadopol razvanpapadopol added the [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging label Oct 29, 2020
@ramonjd
Copy link
Member Author

ramonjd commented Oct 29, 2020

We already replaced 'Complete setup' label with 'Launch' also for Gutenboarding in #46558 so at this moment we should just wait for strings to be translated and then merge this PR and #46817 🚢

Thanks for the info @razvanpapadopol and @ciampo! All good, I'll put a string freeze on this PR and keep it on ice :) thanks for adding the string freeze 😇

@ciampo ciampo merged commit 6a9bbcc into master Nov 6, 2020
@ciampo ciampo deleted the update/gutenboarding-nux-copy branch November 6, 2020 12:03
@matticbot matticbot removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Status] String Freeze Add the [Status] String Freeze label to your PR to ensure new strings are translated before merging labels Nov 6, 2020
@a8ci18n
Copy link

a8ci18n commented Nov 6, 2020

Translation for this Pull Request has now been finished.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Coming Soon [Goal] New Onboarding previously called Gutenboarding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants