Extend Dashboard Sharing feature tour to include settings interface

[aaemnnosttv]

Feature Description

In #5328, we added a basic feature tour for dashboard sharing which informs an authenticated admin about the added ability to share the dashboard with other users but does not elaborate on how to use it.

This issue is about extending this tour with additional steps to walk the user through the sharing settings interface and better explain how to share the dashboard and or delegate sharing management.

---

Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

The feature tour for dashboard sharing should be extended as follows:

• The tour should get additional steps to walk the user through the sharing settings interface
  • A second step should be added:
    • Title: Manage view access for other roles
    • Body: Grant access to the view-only dashboard for each service for the specific roles you want. Users will see the Site Kit dashboard with only the services that have been shared with them without needing to sign-in with Google.
    • On display, the dashboard sharing settings modal should be toggled open
    • Focus: "Who can view" column
  • A third step should be added but only if the site has multiple admins (otherwise this column will not be displayed/relevant)
    • Title: Share management with other admins
    • Body: By default only the user who connects a service can control who it is shared with. This setting optionally lets these users allow any other admin signed in with Google to manage the roles a service is shared with.
    • Focus: "Who can manage" column
  • On dismiss/completion the dashboard sharing settings modal should be closed
• If the user dismisses the feature tour on the first step (showing settings button) they should still see the remaining tour steps when viewing the sharing settings interface for the first time (i.e. on demand)
  • Dismissing on step 2 or 3 should dismiss the full tour as usual

Note: these could technically be separate tours with shared steps, etc – it is not strictly required that everything is defined in a single tour.

Implementation Brief

• Update DashboardSharingSettingsButton to use the CORE_UI store for the dialogOpen state (e.g. dashboardSharingDialogOpen)
• Update _googlesitekitDashboardSharingData to include the value of hasMultipleAdmins from Has_Multiple_Admins
• Define a new dashboardSharingSettings feature tour
  • The first step should be defined as the "second" step in the ACs
    • The target should use .googlesitekit-dashboard-sharing-settings__main .googlesitekit-dashboard-sharing-settings__column--view to highlight the first row's user role select control
  • If _googlesitekitDashboardSharingData.hasMultipleAdmins is true, the second step (corresponding to the "third" step in the ACs) should be included/appended
    • The target should use .googlesitekit-dashboard-sharing-settings__main .googlesitekit-dashboard-sharing-settings__column--manage to highlight the first row's sharing management control
  • The contexts for this tour should be omitted because it should not be triggered automatically by view context, but only by its inclusion in the dashboardSharing tour or on its own, on-demand
• Update the dashboardSharing tour to import the new dashboardSharingSettings tour definition, appending its steps to its own
  • Define the callback function to respond to the tour events appropriately:
    • If transitioning to the 2nd step, set the dashboardSharingDialogOpen state to true
    • If transitioning to the 1st step, set the dashboardSharingDialogOpen state to false
    • When exiting the tour, ensure dashboardSharingDialogOpen is set to false again
    • Keep track of visited tour steps, if step 2 was reached, dismiss the dashboardSharingSettings tour when exiting the tour

• Update feature tour infra to work similar to user surveys
  • Introduce a new initialState.currentTour property, which should hold the definition of the current tour being shown
  • Introduce a new selectors.getCurrentTour (no arguments) that returns the state.currentTour
  • Introduce a new actions.receiveCurrentTour action that sets the state.currentTour
  • Introduce a new actions.triggerTour which takes a tour object and receives the currentTour if there is none currently set
  • Introduce a new actions.triggerTourForView(viewContext)
    • If feature tours are currently on cooldown (areFeatureToursOnCooldown), the action should return early and do nothing else
    • Otherwise, it should take the given viewContext and check all the tours as we do now in resolvers.getFeatureToursForView, and dispatch triggerTour with the first tour that qualifies
  • Introduce a new actions.triggerOnDemandTour(tour) that checks the given tour's requirements (below) before dispatching triggerTour(tour):
    • The tour should not be dismissed
    • Its own checkRequirements function should return true
    • It should not be affected by tour cooldown
  • Update FeatureTours to source the tour it passes to TourTooltips via the new getCurrentTour selector only
    • A new onMount effect should be used to dispatch actions.triggerTourForView(viewContext) to preserve the current behavior of invoking tours for the current viewContext
  • selectors.getFeatureToursForView and its resolver should no longer be necessary
  • Update actions.dismissTour to unset state.currentTour if the given tour slug matches the current tour's slug
  • Update DashboardSharingSettingsButton with a useEffect to triggerOnDemandTour with the dashboardSharingSettings tour object when the dialog is opened
    • Note the dashboardSharing tour will essentially trigger this as part of the changes here but the secondary triggerTour will have no effect because a currentTour will already be set/running

Test Coverage

• Add tests for the new actions and selectors added to the core/user data store.
  • selectors.getCurrentTour
  • actions.receiveCurrentTour
  • actions.triggerTour(tour)
  • actions.triggerTourForView(viewContext)
  • actions.triggerOnDemandTour(tour)

• Update tests for actions.dismissTour
• Remove tests for getFeatureToursForView

QA Brief

This issue extends the existing feature tour for dashboard sharing with additional steps to cover the sharing settings.

• For a site with a single admin, the tour will have 2 steps (1 extra)
• For a site with multiple admins, the tour will have 3 steps (2 extra)
• If the user sees the main tour and dismisses it before they get to the last step, they will see a secondary feature tour when the sharing settings are opened (even if they just saw the first)
  • This secondary tour will only have the extra 1-2 steps as mentioned above
  • They will always see all the extra steps in this tour, not just the ones they didn't see from the first
• It is possible for a user to see a different tour before the main DS tour (enabling the cooldown) and then trigger the on-demand DS settings tour without seeing the first as it is currently. This is probably an edge case we don't need to worry about too much but also could be addressed if we want to with out a large additional effort

Changelog entry

• Extend the Dashboard Sharing feature tour to include steps for the settings interface.

[jimmymadon]

@aaemnnosttv As part of the mini discovery, Mariya's suggested flow is possible. We could also allow the user to click on the Dashboard Sharing Settings button itself as part of 'Step 1' of the tour. Then a new tour would be triggered when the user sees the dialog box for the first time (there is small engineering challenge here). This way we can "dismiss" these two tours independently, in case the user decides to close the first feature tour without opening the settings dialog box.

The engineering challenge here is triggering the second tour when the dialog box opens without adding code outside of the Feature Tours infrastructure. @asvinb suggested looking into the useMutationObserver to check for the addition of the dialog box classes in the DOM. But I think this still requires changes outside the feature tours code. After having a chat with @tofumatt, we still think this won't be easily possible. So we can simply add an optional "isNotTriggeredAutomatically" flag (default false) when defining the feature tour's data array (the one which defines steps and content). This will allow us not to "automatically" execute the startTour logic in the useMount within <TourTooltips> but to do this in the <DashboardSharingSettingsButton> component. This can be further discussed in the IB of this issue.

[wpdarren]

@aaemnnosttv @jimmymadon while testing another dashboard sharing ticket, the feature tour appeared and my instant thought was that it could be more helpful. I feel the feature tour should include a link to the Dashboard Sharing guide. We tell the user what the icon is for, but don't tell them where to find more information.

Would this be an easy update to add a learn more link within the ACs on this ticket?

[aaemnnosttv]

@wpdarren I don't think we want to send the user away from their site as part of the feature tour. The sharing settings have a learn more link within the interface – together with an extended tour I feel this should be sufficient.

The goal of this issue is to make that same tour more informative and part of that is showing them the sharing settings. Perhaps we could also link to the documentation from the last step of the tour but ideally everything should be clear enough without reading additional documentation.

[eclarke1]

@aaemnnosttv following up here, @marrrmarrr replied in Slack saying this looks good - are we ok to move this over to IB now do you think?

[aaemnnosttv]

Thanks @asvinb – this looks pretty well thought out, nice work! A few thoughts for you on some of the details:

• Add spotlightClicks: true to the first and only step. Setting spotlightClicks to true will allow the user to click the dashboard sharing settings icon to reveal the modal.

The intention here is for the modal to be opened when transitioning to the second step of the tour. We had discussed the idea of guiding the user to click it as part of the tour but it should be easier to do this automatically and also keeps things consistent by limiting interactions to the tour tooltips which is consistent with all other tours we have.

• Set the zIndex (new property) to 10001 within joyrideStyles.options since the tour tooltips should be on top of any modal. In our case, the dashboard sharing settings modal has a z-index of 10000.

Does this apply to just the tooltips or also the focus area floater?

• Within the openDialog function, after setDialogOpen is called, dispatch the dismissTour action from core/user data store, with dashboardSharing as parameter. This means as soon as the button is clicked, we dismiss the first tour.

This will probably need to change from using local state to CORE_UI in the datastore so that we can toggle it open as part of the tour (as mentioned before).

Second, I'm not sure that this is needed – most users should see the tour before they have a chance to trigger it, but since we have a secondary tour for the settings which will be triggered on first use, we can simply use this in the requirements check for the first tour: if the sharing settings tour has been dismissed, then don't show the first tour.

• Since we can't use the useSelect hooks when registering new feature tours, for e.g to check if there are multiple admins, I think the easiest solution would be to have separate feature tours, i.e one for multiple admins and one for single one, and load them accordingly.

We shouldn't need to create separate tours for the with/without multiple admins case, as the main difference here is just an additional step. We'd only need to be able to determine if there are multiple admins on load to know whether or not we need that extra step or not.

Thinking about having them separate for a minute, it could be beneficial to show the multi-admin tour if a single admin site later added a second admin. It probably wouldn't be beneficial for a multi-admin site to later see the single-admin tour if they later became the only admin. We could consider this as an enhancement in the future, I think it increases the scope a bit more than necessary for our needs at this point though.

Let's use a single tour for the settings with a variable number of steps for now. We can expose hasMultipleAdmins in a global for now if needed. In the future, it would probably be good to allow steps to be an async function that has access to registry, similar to tour.checkRequirements but I don't think need to do that just yet.

• Add tests for the new actions and selectors added to the core/user data store.

This would be the minimum as per usual, but if we're making changes to the existing feature tour infrastructure, this should be accounted for as well.

[jimmymadon]

The intention here is for the modal to be opened when transitioning to the second step of the tour. We had discussed the idea of guiding the user to click it as part of the tour but it should be easier to do this automatically and also keeps things consistent by limiting interactions to the tour tooltips which is consistent with all other tours we have.

@aaemnnosttv I originally suggested using spotlightClicks and updating the content of step 1 to encourage the user to click the button as that would be simpler. Since we will be using two tours, the first and only step of tour 1 will have the "Got it!" primary close button which would normally close the tour. We don't have the "Next" button here. Changing this behavior to automatically open the settings dialog would entail adding extra logic within the step's callback() and somehow change the locale text for the "Got it!" button to say "Try it now!" only for this particular tour (which would be a bit hacky).

[jimmymadon]

Does this apply to just the tooltips or also the focus area floater?

@aaemnnosttv Just setting it to joyrideStyles suffices based on testing this in a POC / experimental branch courtesy @asvinb!

This will probably need to change from using local state to CORE_UI in the datastore so that we can toggle it open as part of the tour (as mentioned before).

The IB currently suggests that the <DashboardSharingSettingsButton> will trigger a state change to add the ad-hoc dashboard-sharing-settings tour (using the new START_TOUR action which updates pendingTourSlug state ) to the datastore. This logic should probably remain in the core/user store if we go down this route. However, I have spoken to @asvinb and I will see if we can have the dashboard-sharing-settings tour available in the datastore already, just not rendered by default. This should also simplify the extra logic of 'which tour should be rendered next' in the <FeatureTours> component. In that case, we can use the core/ui store to simply switch the tour to be running like we do now automatically in <TourTooltips> (but it somehow needs to be 'next in line'). Keeping this ad-hoc tour 'next in line' but not displaying it could potentially block other future feature tours which need to show up on the dashboard before the user clicks on the DS Settings button.

I think the rest of your feedback is actionable and I will try these out in the POC branch and update the IB!

[tofumatt]

Regarding the IB here… I'm not a big fan of "overloading" the context argument with a "magic" ON_DEMAND viewContext. This isn't a real value and realistically ends up representing a feature-tour that isn't dependant on a viewContext, but instead is triggered by something else entirely.

I'd rather us add a new property for feature tours like showOnDemand and make viewContext for a feature tour optional in the presence of the showOnDemand property. Functionally it would behave the same, but that way we don't have any magic values.

In addition to that point though, I think I'm a bit confused by the IB here. There's mention of extending a feature tour with data from another based on conditions, but I'm not clear on why that's needed. I'd think we'd have another feature tour triggered when opening the Dashboard Sharing modal/dialog, and clicking "next" on the "first" feature tour (eg. the first "step" of this multi-tour tour) would open the Dashboard Sharing modal/dialog.

I think I'm not quite following the IB here and how it all fits together. I know @jimmymadon is not available to review these comments now, but maybe @asvinb or @aaemnnosttv could clarify this a bit for me? I'm not quite clear on what exactly is changing here and why some of these changes are even needed 🤔

[eclarke1]

Unfortunately @asvinb is also resourced to other projects for a few weeks too, so we may need @aaemnnosttv and @tofumatt to own this one and push it through to the finish line - WDYT?

[tofumatt]

I'm a bit confused as the proof-of-concept PR doesn't look anything like the IB from what I can tell, but I'm going to check out the PR and see how it works.

[tofumatt]

Overall the PR looks good except for some intermittent glitch-y-ness when finishing the tour on the "third"/"last" step:

I'm going to try tweaking the proof-of-concept though because otherwise it seems to be working well. But the proof-of-concept makes no reference to a lot of stuff (like the ON_DEMAND viewContext) so I'm wondering if it's out-of-date and was meant to be adjusted but it didn't take? 🤔

[aaemnnosttv]

I'm not a big fan of "overloading" the context argument with a "magic" ON_DEMAND viewContext. This isn't a real value and realistically ends up representing a feature-tour that isn't dependant on a viewContext, but instead is triggered by something else entirely.

@tofumatt, I was thinking we might want to be able to use a given tour on load (as it is now) or on-demand in some cases. The current tour requirements check requires tour.contexts but I think this shouldn't be needed given the approach outlined here checks the requirements separately for an on-demand tour. I think the addition

I'd rather us add a new property for feature tours like showOnDemand and make viewContext for a feature tour optional in the presence of the showOnDemand property. Functionally it would behave the same, but that way we don't have any magic values.

Yes I thought about that as well, but I think such a property shouldn't be necessary as on-demand tours are checked differently as defined.

There's mention of extending a feature tour with data from another based on conditions, but I'm not clear on why that's needed. I'd think we'd have another feature tour triggered when opening the Dashboard Sharing modal/dialog, and clicking "next" on the "first" feature tour (eg. the first "step" of this multi-tour tour) would open the Dashboard Sharing modal/dialog.

The idea is that in the user's experience, it's only one tour so long as they see both parts. Implemented as separate tours that are triggered seamlessly, the experience is unintuitive because you'd first see a tour with one step and then immediately see a second tour with one or two steps. This way, the user sees (and interacts with) the first tour as a more comprehensive tour from the beginning, and is able to navigate back and forth to/from any step until it is dismissed (like others). As separate tours, they'd be stuck in the second, unable to go back to the first once triggered. This is also why the ACs call for a second and conditional third step to be added to the first tour rather than simply triggering a separate tour immediately after dismissing the first.

That's why the first is more of a composite tour – we only show the settings part of it on demand if they don't reach that part of the tour in the first.

I'm a bit confused as the proof-of-concept PR doesn't look anything like the IB from what I can tell, but I'm going to check out the PR and see how it works.

The current IB is a bit different and came out of conversations I had with @jimmymadon last week, but he didn't have time to update it before he left which is why I've taken over here 😄

---

I've updated the IB to remove the special ON_DEMAND context, but the rest should be good to go I think. Let me know if you have any other questions 👍

[tofumatt]

Ah, okay, I think that clarifies everything, thanks!

The IB looks good now, thanks for fixing it up and adding the comments 👍🏻

IB ✅

[wpdarren]

QA Update: ❌

Update: I will create a ticket for the minor observation (+ include it as a known issues) and get this moved forward.

@tofumatt @aaemnnosttv a very minor observation. On scenario 3 in the QAB, where the user dismisses the tour and they click on the dashboard sharing settings icon, the tour is triggered again - when the tour appears on 'add user role' the highlighted area slightly overlaps the icon. It's more of an issue on Safari but noticed it on Chrome too.

Can I also assume that like the other feature tours, this does not appear on small screen sizes like mobile phones and tablets in portrait mode? It does appear on tablets in landscape mode.

Side note: the colour of the tour tooltips are black with GM2+ styles, and this is an issue under investigation from UX because it should really be appearing as the primary green. I wanted to add that for accessibility when using the tab, arrow keys and enter, it's not obvious where you are on the tour, so hopefully the colour change will improve that.

[eclarke1]

Just a note regarding the background colour of the feature tour, I have added a comment here #5254 (comment) for @aaemnnosttv to kindly review and add today please.

[wpdarren]

QA Update: ✅

@aaemnnosttv @tofumatt @eclarke1

There are a few issues but I do not feel that they impact bug bashing or are launch blockers:

1. If a user updates Site Kit from an earlier version (e.g. 1.38.0) when visiting the dashboard for the first time, they will receive a feature tour for unified dashboard. The Dashboard sharing tour does not get triggered. When the user clicks the dashboard sharing settings icon, then the second part of the tour does get triggered. It just misses the first step of the tour.

Created ticket here and added to bug bash board.

2. I feel the language could be improved on the 2nd step of the feature tour. It references service but within the UI we say that they are products. It does match the language included in the AC.

Have added this to the bug bash board for discussion.

3. The UX/UI issue I mentioned in this comment I have created a ticket and added it on to the known issues board.

Other than these areas, this is good to go, so have approved.

Verified:

• For a site with a single admin, the tour has 2 steps.
• For a site with multiple admins, the tour has 3 steps.
• When the user dismisses the main tour before they get to the last step, they see a secondary feature tour when the sharing settings are opened. This secondary tour has the extra steps.
• Tested on all of our supported browsers using Browserstack.

Screenshots

[felixarntz]

Other than the already created follow-up issues #5520 and #5521 this looks solid ✅