-
Notifications
You must be signed in to change notification settings - Fork 293
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
Prevent dashboard feature tours from being triggered during module setup #3187
Comments
I think this is something to evaluate on a per-tour basis. For help specifically, I think it makes sense to allow here as module setup is probably one of the areas where users encounter trouble. However, the dashboard and page dashboard view contexts shouldn't include module setup IMO. Perhaps what we should do is move module setup into a dedicated page rather than reusing the dashboard URL for this. We can't really use the module-specific dashboard screens for this since not every module has one, but most have a set up flow. So in this case, I think the problem is more that the dashboard entrypoint |
@aaemnnosttv Sounds good. Maybe let's refocus this issue on splitting out the responsibility of module setup from the dashboard, e.g. via a |
@aaemnnosttv @eugene-manuilov @tofumatt Do we still think this is an idea worth pursuing at this point? We've been occasionally talking about working more towards a single page application, and this issue specifically would counter that tendency. Should we maybe close this instead? |
@felixarntz As is, I don't think we need a separate JS asset to solve this problem. The simpler change would be to use a different |
Yep, I agree with Evan. Let's solve it using a simpler way instead of closing it. |
@aaemnnosttv @eugene-manuilov Sounds like a plan - I've added ACs accordingly (also a lot simpler!). |
Sorry @eugene-manuilov for the accidental assignment there - mouse slip! |
IB ✔️ |
@aaemnnosttv as per our conversation on Slack last week, am I assuming correctly that this cannot be tested until #4069 is in develop? I have tried to trigger the |
@wpdarren it should be testable now, but you'll need to disable the overriding of feature flags since this one is no longer controllable via the tester plugin. You may still need the tester plugin active though to force the older initial version. Does that make sense? |
@aaemnnosttv okay, I have run through this again, but no success at getting the Here are the steps that I took.
Any ideas? |
@wpdarren try clearing any dismissed tours you may have by deleting |
@aaemnnosttv I looked at that and |
Feature Description
We probably need to update tooltip tours to avoid popping up on the module setup pages. It is not a big deal and a user can always close it for sure, but it would be more convenient if we just don't display tooltip tours there.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
VIEW_CONTEXT_MODULE_SETUP
with value "moduleSetup" should be added.viewContext
prop ofRoot
when theModuleSetup
component will be used within (i.e. withingooglesitekit-dashboard.js
andgooglesitekit-module.js
).Implementation Brief
Inside
assets/js/googlesitekit/constants.js
:VIEW_CONTEXT_MODULE_SETUP
, with value "moduleSetup".Inside
assets/js/googlesitekit-dashboard.js
:VIEW_CONTEXT_MODULE_SETUP
in addition to the existingVIEW_CONTEXT_DASHBOARD
setupModuleSlug
is truthy, pass the newVIEW_CONTEXT_MODULE_SETUP
constant to<Root>
as theviewContext
prop.setupModuleSlug
is falsy, passVIEW_CONTEXT_DASHBOARD
to<Root>
as theviewContext
prop.Inside
assets/js/googlesitekit-module.js
:VIEW_CONTEXT_MODULE_SETUP
in addition to the existingVIEW_CONTEXT_MODULE
setupModuleSlug
is truthy, pass the newVIEW_CONTEXT_MODULE_SETUP
constant to<Root>
as theviewContext
prop.setupModuleSlug
is falsy, passVIEW_CONTEXT_MODULE
to<Root>
as theviewContext
prop.Test Coverage
Visual Regression Changes
QA Brief
Changelog entry
The text was updated successfully, but these errors were encountered: