Implement welcome guide modal #18041

noisysocks · 2019-10-21T05:57:52Z

Closes #16315.

Implements the welcome guide modal described in #16315 (comment).

What's changed

There's a new Guide component in @wordpress/nux which implements the guide modal in a reusable way. This is marked experimental so that we can potentially move this to @wordpress/components in a follow-up PR.
WelcomeGuide in @wordpress/edit-post will use Guide to display a welcome guide for the block editor if the user has not previously disabled tips. This replaces all of our instances of the DotTip component. We can potentially move away from using hasDisabledTips in a follow-up PR.
There's a new Welcome Guide button in the More menu which allows the user to open the guide again. This replaces the Enable Tips setting in the Options modal.

How to test

Unit and E2E tests are included. To manually test the upgrade process, follow these steps:

On master, open the block editor and ensure that you have dismissed tips.
Check out this branch and open the block editor. The welcome guide should not appear.
Rest your browser's localStorage to simulate being a new user.
Refresh the page. The guide should appear.
Dismiss or complete the guide.
Refresh the page. The guide should no longer appear.

noisysocks · 2019-11-06T07:02:25Z

There's still lots to do here, but it would be good to get some early design feedback 🙂

packages/edit-post/src/components/welcome-guide-modal/index.js

enriquesanchez · 2019-11-12T22:25:53Z

Hey @noisysocks! 👋

Happy to help with design feedback. How can I get this to show up on my local install?

noisysocks · 2019-11-14T04:17:19Z

Thanks @enriquesanchez! You can check out the branch and then open the Welcome Guide from the More menu:

noisysocks · 2019-11-18T11:18:12Z

packages/edit-post/src/components/welcome-guide-modal/index.js

+	return (
+		<Modal
+			className="edit-post-welcome-guide-modal"
+			shouldCloseOnClickOutside={ false }


I've disabled this to work around an issue where the modal is immediately dismissed on page load because the post's title field receives focus. Let me know if you can think of a better alternative!

packages/edit-post/src/components/welcome-guide-modal/index.js

aduth · 2019-11-19T14:57:15Z

packages/edit-post/src/components/welcome-guide-modal/index.js

+						{ __experimentalCreateInterpolateElement(
+							__( 'All of the blocks available to you live in the Block Library. You’ll find it wherever you see the <InserterIconImage /> icon.' ),
+							{
+								InserterIconImage: (


I like how you chose to name the property to align with the component name, but in-so-doing it occurs to me whether we ought to have any coding guidelines around the names we choose for these interpolated keys? cc @nerrad

I think this is a question best answered by those working more closely with translators.

One of the intents behind the interface is to provide additional guidance to translators on what a tag represents and signal that it shouldn't be translated. With WordPress core in particular, I think generally translators know to leave tags alone (although that has been a smaller subset of tags I think).

I think others might be better equipped to detail what standards we should have. cc @ocean90 , @swissspidy or @akirk , do you have any guidance here?

I am huge proponent of "speaking" component placeholders. It's a very easy way to convey to translators what is being substituted here. InserterIconImage sounds good to me.

How and where would be put coding guidelines for this?

I like this proposal. Assuming that it's copied to all translations it would work great 👍

For the full picture, there are some drawbacks discussed in the PR which test this new API. The discussion triggered by @johnbillion starts with this comment: #18623 (comment).

@akirk do you suggest that the following:

{ __experimentalCreateInterpolateElement( __( 'While writing, you can press <kbd>/</kbd> to quickly insert new blocks.' ), { kbd: <kbd /> } ) }

could be easier refactored to:

{ __experimentalCreateInterpolateElement( __( 'While writing, you can press <Shortcut /> to quickly insert new blocks.' ), { Shortcut: <kbd>/</kbd> } ) }

TimothyBJacobs · 2019-11-19T17:27:18Z

Would it be possible for the <Guide /> component to be a part of NUX? This looks like something that'd be awesome to use on plugin admin screens as well. But since <Guide /> is in editPost it'd be difficult to use.

Separately, it looks like #12632 would still be effected by this tips redesign. AFAICT, the close button here is dismissing all guides instead of just the current "guide".

noisysocks · 2019-11-20T10:23:32Z

Would it be possible for the <Guide /> component to be a part of NUX? This looks like something that'd be awesome to use on plugin admin screens as well. But since <Guide /> is in editPost it'd be difficult to use.

Good suggestion! I'll look into this.

@mapk: Do you think a guide component would be useful outside of WP Admin? If so, then I think we should consider putting Guide and/or GuideModal into @worpdress/components so that we get to use the existing infrastructure there e.g. storybooks.

Separately, it looks like #12632 would still be effected by this tips redesign. AFAICT, the close button here is dismissing all guides instead of just the current "guide".

Thanks for pointing me to #12632. I'll look into moving away from areTipsEnabled(). My preference is to make Guide a dumb component and leave it up to the plugin developer to decide when to show it.

mapk · 2019-11-20T16:33:35Z

@mapk: Do you think a guide component would be useful outside of WP Admin? If so, then I think we should consider putting Guide and/or GuideModal into @worpdress/components so that we get to use the existing infrastructure there e.g. storybooks.

YES! Adding this to @wordpress/components sounds reasonable to me. Allowing other developers to use it could be amazing.

mapk · 2019-11-20T17:33:34Z

I really like how this is shaping up! Great work. Here's some design feedback:

1. First image needs to be centered better.

The webpage in the graphic feels like it's not quite centered. Even the blocks in the webpage don't look centered.

2. First two graphics should be equal width.

I think this may help visually if both graphics in the first two images are equal width within the blue container.

3. Should we capitalize certain things?

"Block Editor" or "Block Library" seem like proper nouns. For example, we capitalize Block Library in the sentence, but not in the heading of the third screen.

4. Second screen text addition

Should we mention that you can move the block toolbar to be fixed at the top of the screen for less visual distractions?

5. This icon can probably use some visual cleanup.

Also lets make the triangle white to match the popover graphic too. Right now it's grey.

I think these last few changes will dial it in beautifully.

enriquesanchez · 2019-11-20T22:49:19Z

Hey @noisysocks! This is looking great! 🎉

Visuals

Would it be possible to align the close button so it has the same space around like the previous and next/get-started buttons?

I also noticed that the hover state of the page control buttons looks a bit off. It looks like the right border is missing half a pixel (I'm on a retina display).

Accessibility

I was able to interact with the welcome guide using only my keyboard 👍

I also did a quick test with Voice Over and had a couple of issues:

The page control buttons could use an aria-label so it's communicated to assistive technology what they're for. I was expecting to hear something like "page 1 of 3".

I'm thinking these could be grouped on a ul so they're part of the same group. Following suggestions on how to markup content sliders from the Inclusive Components pattern library, it could be something like:

<ul aria-label="Welcome guide controls">
    <li><button aria-label="Page 1 of 3"></button></li>
    <li><button aria-label="Page 2 of 3"></button></li>
    <li><button aria-label="Page 3 of 3"></button></li>
</ul>

I don't think the images need an alt description, because the accompanying text does a good job of describing each page/topic. That being said, they should have an empty alt = "" so that they're ignored by AT.

Overall, this is looking great! Let me know what you think of my comments and suggestions.

enriquesanchez · 2019-11-20T23:08:35Z

@noisysocks where are you getting the images from? Let me know if you need help with Mark's suggestions on alignment and size.

noisysocks · 2019-11-21T09:06:13Z

Thanks @mapk and @enriquesanchez! I'll work through your suggestions.

@noisysocks where are you getting the images from? Let me know if you need help with Mark's suggestions on alignment and size.

I would absolutely love it if you could update the images for me! I got the SVGs from @jasmussen, who I think exported them from the 'Tips Iteration' project in the WordPress.org Figma team.

enriquesanchez · 2019-11-21T18:56:53Z

Hey @noisysocks! (I never get tired of your username 😃)

Here are the new SVGs with Mark's suggested changes:

illustrations.zip

enriquesanchez · 2019-11-21T19:01:19Z

Wait, I noticed the add block icon on 'illustration 3.svg` was still showing some weird artifacts.

Here's a cleaner version:
illustration 3_.svg.zip

noisysocks · 2019-11-22T11:23:41Z

First image needs to be centered better.

First two graphics should be equal width.

This icon can probably use some visual cleanup.

👍 I updated the images in d8cee82. Thanks for fixing those up @enriquesanchez!

Should we capitalize certain things?

👍 I capitalised 'Block Editor' and 'Block Library' in 745fdd4.

Second screen text addition

I'll defer to you all on this one. Let me know what you'd like the copy to be and I can update it.

Would it be possible to align the close button so it has the same space around like the previous and next/get-started buttons?

I updated the alignment in 47c2879, though I wonder if doing this makes the X button look too much like 'content' instead of 'chrome'? Let me know what you think.

The page control buttons could use an aria-label so it's communicated to assistive technology what they're for. I was expecting to hear something like "page 1 of 3".

👍 I updated the page controls to use the markup you suggested in 92e1773.

I don't think the images need an alt description, because the accompanying text does a good job of describing each page/topic. That being said, they should have an empty alt = "" so that they're ignored by AT.

The images already have an empty alt="" attribute.

noisysocks · 2019-11-25T15:46:00Z

Separately, it looks like #12632 would still be effected by this tips redesign. AFAICT, the close button here is dismissing all guides instead of just the current "guide".
Thanks for pointing me to #12632. I'll look into moving away from areTipsEnabled(). My preference is to make Guide a dumb component and leave it up to the plugin developer to decide when to show it.

Let's look at making the welcome guide use something other than areTipsEnabled in a separate PR. There are some details to consider there and I want us to have the opportunity to properly think it through. We also need to consider whether or not we want to maintain or deprecate the @wordpress/nux package. #12632 can track that work.

YES! Adding this to @wordpress/components sounds reasonable to me. Allowing other developers to use it could be amazing.

I've left Guide in the @wordpress/nux package for now because it requires isViewportMatch to determine whether or not to display the 'Get started' button inline with the content. Let's consider moving Guide to @wordpress/components in a follow-up PR—I've marked Guide as experimental for now so that we can do this.


        Welcome guide: Close guide when user clicks on overlay

joedolson · 2019-12-06T17:19:02Z

The modal is not labelled. This is in part a problem with this modal, but should probably be corrected in the component: the Modal component allows the component to be misused.

We should enforce a label for the modal element with role=dialog within the e2e tests, because all modals must be labelled. Additionally, the modal component should enforce 'role="document"' on the content of all modal dialogs. This isn't strictly required on all modals, but the component can't know what type of content a modal will contain, this will give the best overall experience.

There are usability problems with having this modal open automatically on page load.

As it is (unlabeled), it's going to be confusing for some users.
It disrupts the flow for users who wish to use Gutenberg immediately.
Users who reflexively dismiss modals will not easily be able to get it back.

One possible way that this could be made available is to add a prominent button for users to launch the Welcome Modal if they are detected as new Gutenberg users. The modal, in that case, should have two cancel actions: one that simply closes the modal, and one that dismisses it persistently.

As a second issue, the method for assessing whether a user is "new" to Gutenberg is significantly flawed: the usage of local storage for this means that a user will be new on every device they use, on every browser they use, and on every new site they launch. Secondarily, because the detection appears to be based on the positive presence of 'modal dismissed' setting in local storage, it will essentially be impossible to permanently disable it; anytime the local storage is flushed it will come back. (See #15105)

aduth · 2019-12-17T15:58:33Z

packages/components/src/guide/style.scss

+		}
+	}
+
+	&__container {


While I think this convention certainly makes things more concise, in practice I find it adds difficulty to a very common use-case of searching for a class name in the codebase. To try to search for "components-guide__container" would not yield this as a result.

While it requires a couple more characters to type, I would personally advocate to use the fully expanded form in source:

.components-guide__container {

aduth · 2020-03-02T18:50:52Z

packages/components/src/guide/finish-button.js

+	// Focus the button on mount if nothing else is focused. This prevents a
+	// focus loss when the 'Next' button is swapped out.
+	useLayoutEffect( () => {
+		if ( document.activeElement === document.body ) {


In #20594, I am encountering an issue where we make certain assumptions about what it means for "no focus" to exist. It appears that the value of activeElement may differ depending on browser:

When there is no selection, the active element is the page's <body> or null.

https://developer.mozilla.org/en-US/docs/Web/API/DocumentOrShadowRoot/activeElement

I mention it here, since this may not cover all focus losses, notably those in Internet Explorer. Specifically, this condition is only checking for the first of these two possible "no active element" scenarios.

It might be something where we want to provide some utility, e.g. wp.dom.hasActiveElement, since it seems to be an easy issue to overlook.

Edit: I confirmed that there is a focus loss which occurs when reaching the last page of the guide in Internet Explorer:

Fix at #20599

noisysocks added [Type] Enhancement [Feature] NUX labels Oct 21, 2019

noisysocks force-pushed the add/welcome-guide branch from 9aed3a9 to bab8f7b Nov 6, 2019

noisysocks added the Needs Design Feedback label Nov 6, 2019

noisysocks reviewed Nov 12, 2019

View changes

packages/edit-post/src/components/welcome-guide-modal/index.js Outdated Show resolved Hide resolved

noisysocks force-pushed the add/welcome-guide branch 3 times, most recently from 4dfdc1c to bc79748 Nov 15, 2019

noisysocks reviewed Nov 18, 2019

View changes

packages/edit-post/src/components/welcome-guide-modal/index.js Outdated Show resolved Hide resolved

noisysocks reviewed Nov 18, 2019

View changes

packages/edit-post/src/components/welcome-guide-modal/index.js Outdated Show resolved Hide resolved

aduth reviewed Nov 19, 2019

View changes

enriquesanchez mentioned this pull request Nov 19, 2019

NUX: Consider making "close" button hide the current guide instead of disabling tips completely #12632

Closed

noisysocks force-pushed the add/welcome-guide branch from bc79748 to 92e1773 Nov 22, 2019

noisysocks force-pushed the add/welcome-guide branch from 92e1773 to 99c80cc Nov 22, 2019

noisysocks marked this pull request as ready for review Nov 25, 2019

Welcome guide: Close guide when user clicks on overlay

Loading status checks…

7b8773d

noisysocks force-pushed the add/welcome-guide branch from ef97cae to 7b8773d Dec 2, 2019

noisysocks merged commit 574e8ea into master Dec 2, 2019
2 checks passed

2 checks passed

pull-request-automation
Details

Travis CI - Pull Request Build Passed
Details

noisysocks deleted the add/welcome-guide branch Dec 2, 2019

youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019

shaunandrews mentioned this pull request Dec 9, 2019

Re-enable Gutenberg tips on WordPress.com and update the copy to be specific and relevant for new users. #38299

Closed

This was referenced Dec 11, 2019

Welcome Guide: modal should be labelled #19078

Closed

Welcome Guide: improve method for assessing whether a user is "new" #19079

Closed

Copons mentioned this pull request Dec 12, 2019

WPCOM Block Editor: Remove disable-nux-tour #38369

Closed

aduth reviewed Dec 17, 2019

View changes

aduth mentioned this pull request Dec 17, 2019

Edit Post: Fix Welcome Guide modal display for Internet Explorer #19201

Merged

noisysocks mentioned this pull request Dec 19, 2019

NUX: Customize aria-label #12631

Closed

This was referenced Jan 15, 2020

Update dependency @wordpress/editor to v9.10.0 ampproject/amp-wp#4087

Merged

Add missing npm dependency ampproject/amp-wp#4150

Merged

jorgefilipecosta added the Needs Dev Note label Feb 4, 2020

jorgefilipecosta mentioned this pull request Feb 12, 2020

Block Editor Dev Notes 5.4 #20185

Closed

22 of 23 tasks complete

aduth reviewed Mar 2, 2020

View changes

aduth mentioned this pull request Mar 2, 2020

Components: Fix focus loss for Guide Finish button in IE #20599

Open

WordPress / gutenberg

Join GitHub today

Implement welcome guide modal #18041

Implement welcome guide modal #18041

Conversation

noisysocks commented Oct 21, 2019 • edited

What's changed

How to test

This comment has been minimized.

noisysocks commented Nov 6, 2019

This comment has been minimized.

enriquesanchez commented Nov 12, 2019

This comment has been minimized.

noisysocks commented Nov 14, 2019

This comment has been minimized.

noisysocks Nov 18, 2019

This comment has been minimized.

aduth Nov 19, 2019

This comment has been minimized.

nerrad Nov 19, 2019

This comment has been minimized.

akirk Nov 28, 2019

This comment has been minimized.

gziolo Nov 28, 2019

This comment has been minimized.

gziolo Nov 28, 2019 • edited

This comment has been minimized.

TimothyBJacobs commented Nov 19, 2019

This comment has been minimized.

noisysocks commented Nov 20, 2019

This comment has been minimized.

mapk commented Nov 20, 2019

This comment has been minimized.

mapk commented Nov 20, 2019 • edited

1. First image needs to be centered better.

2. First two graphics should be equal width.

3. Should we capitalize certain things?

4. Second screen text addition

5. This icon can probably use some visual cleanup.

This comment has been minimized.

enriquesanchez commented Nov 20, 2019

Visuals

Accessibility

This comment has been minimized.

enriquesanchez commented Nov 20, 2019

This comment has been minimized.

noisysocks commented Nov 21, 2019

This comment has been minimized.

enriquesanchez commented Nov 21, 2019

This comment has been minimized.

enriquesanchez commented Nov 21, 2019

This comment has been minimized.

noisysocks commented Nov 22, 2019

This comment has been minimized.

noisysocks commented Nov 25, 2019

This comment has been minimized.

joedolson commented Dec 6, 2019

This comment has been minimized.

aduth Dec 17, 2019 • edited

This comment has been minimized.

aduth Mar 2, 2020 • edited

This comment has been minimized.

aduth Mar 2, 2020

noisysocks commented Oct 21, 2019 •

edited

gziolo Nov 28, 2019 •

edited

mapk commented Nov 20, 2019 •

edited

aduth Dec 17, 2019 •

edited

aduth Mar 2, 2020 •

edited