-
Notifications
You must be signed in to change notification settings - Fork 2k
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
WPCOM Block Editor: Remove disable-nux-tour #38369
Conversation
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. |
Two things before I test this:
|
Yes per site, per user, per browser
Yes If we want to avoid that I'd recommend our own NUX redux store/UI backed by an API endpoint that can keep track of this. |
Code looks good otherwise for re-enabling the tour |
This sounds like a blocker. |
Yes, agreed. I think:
Is a blocker, we need to limit it to new users from the time in launches, otherwise it looks bad. |
EDIT: so I think this is what happens. Core Guten dispatches
To display the tips or the welcome guide, the NUX checks for that What does this mean in WPCOM?
1, 2, and 3 are the correct behaviour imho; 4 is debatable. |
While not ideal, I think this is more of an edge case. People clear localStorage when they're troubleshooting an issue, and in that case, it's not unreasonable to think that they'll have to reset some parts of the UI (i.e. sidebar open/close), esp. if it's still dismissible. |
The list above if correct seems perfectly fine to ship to me. |
@michaeldcain well, "clearing" localStorage would also happen when you change browser or device. @apeatling it's a chore to test, but y'all feel free to 😛 |
Just noting that we disabled NUX to cover exactly these scenarios: #32472 Back then, we didn't want those sites to see the NUX. |
To clarify, we didn't want the NUX to show up on every new site (especially if the user already dismissed it on an existing site). |
This PR is supposed to be a stopgap though, a temporary thing while we work on a per-user (instead of per-site) custom NUX, which also involves API changes and so it will take some time to land. Imho it's preferable to have the guide potentially multiple times for the same user, than to have the user disoriented by the block editor. |
733b362
to
ebe622e
Compare
This is the behavior I am seeing when testing on the sandbox. This all seems reasonable to me. It might be slightly annoying for someone who is consistently changing to new devices or creating all sorts of new sites, but I think the benefit of having this show up for new editor users outweighs that slight annoyance.
I agree. |
Just landed #38248, so the Welcome Guide should be visible now to all new sites and users who never used the block editor before on their sites. |
Changes proposed in this Pull Request
disable-nux-tour
override from the WPCOM Block Editor integration.We were disabling it for several reasons: it was on a per-site basis, so users with many sites would get annoyed by it; it had wonky positioning on WPCOM because of its forced fullscreen mode.
Core Gutenberg recently changed the NUX from
DotTip
(tooltips relatively positioned to their container element) to aWelcomeGuide
("multi-step" modal with text and images).Eventually WPCOM will have a fully customized Welcome Guide instead of Core's, but for now, reenabling the vanilla Core NUX is better than nothing. 🙂
Testing instructions
Pre-requisites:
widgets.wp.com
.Test:
This is because the new Core NUX is available starting from Gutenberg 7.1.0, which as of today is not in WPCOM Production yet.
Part of #38299