google / google/site-kit-wp

Feature tours are dismissed on crash (take two).

Open
#7,281 8 comments 0 reactions 3 assignees Assigned to @eugene-manuilov View on GitHub
P1 Type: Infrastructure
Dominant language
JavaScript
Stars
1.4k
Forks
383
Avg merge
4d 14h
Merged PRs (30d)
77

Description

## Bug Description

**Note, this is a redo of https://github.com/google/site-kit-wp/issues/7036, as the IB initially specced for the issue proved not to be entirely effective and an alternative solution was not readily apparent. Therefore we're starting afresh with a new version of the issue.**

As noted in #7031, feature tours can be automatically dismissed due to finishing unexpectedly when a tour step's target is not found. This is most likely due to some data that didn't finish loading which the step target depends on before it will be present.

We should adjust our core tour infrastructure so that tours are only (permanently) dismissed when completed or dismissed by the user.

## Acceptance criteria

* Feature tours should only be dismissed by the following user interactions:
* when completing the tour by clicking "Got it" on the last step
* closing the tour by clicking the "X"
* pressing the Esc key while the tour is active
* Specifically, a tour that ends without completing, due to a missing element for a step should not cause the tour to be dismissed automatically
* This should be the case regardless of the position, within the sequence of steps, of the step with the missing element (see https://github.com/google/site-kit-wp/issues/7036#issuecomment-1619544162).

## Implementation Brief

* In the `assets/js/components/TourTooltips.js`
* Remove `changeStep` callback, and `stepIndex` selector, and it's usage in `Joyride` component. So step switch can be handled internally by component, as there doesn't seem to be need for manual override of steps - no delay, or waiting for specific things is implemented in tours.
* Introduce a new function, `closeTourWithoutDismissal`, which will handle the tour closure without dismissal. Use the existing `endTour` function as a reference. To close tour set `runKey` value to `false` e.q. `setValue( runKey, false );`
* in `handleJoyrideCallback` callback
* Exclude `EVENTS.TARGET_NOT_FOUND` from [`shouldChangeStep`](https://github.com/google/site-kit-wp/blob/019635e605e68078aff6fc6f7c1d224a75e1349a/assets/js/components/TourTooltips.js#L202-L204) check
* Introduce `shouldCloseTour` for `EVENTS.TARGET_NOT_FOUND` checks, use `shouldChangeStep` as reference
* Add `hasMissingTarget` to verify if the current step's target is missing, e.g. `const hasMissingTarget = type === EVENTS.TARGET_NOT_FOUND;`
* Refactor the conditional check for [tour ending and step change](https://github.com/google/site-kit-wp/blob/019635e605e68078aff6fc6f7c1d224a75e1349a/assets/js/components/TourTooltips.js#L222-L226) to check for following:
* If it `shouldChangeStep` or `hasMissingStep` are true, call `changeStep` so store index can be synced
* In case of `shouldCloseTour` and current `index` is on the last step (you can use `steps.length - 1` for this), call `closeTourWithoutDismissal` function
* For `shouldEndTour`, include additional check for `run`, so tour is dismissed only if it is finished and it is still running (missing target on last step will set `run` to `false` via `closeTour`)
* There is basic PoC [PR](https://github.com/google/site-kit-wp/pull/7702) to verify the approach. It can be re-factored/tidied up. Also `changeStep` is not yet removed there

### Tip for debugging

Use `assets/js/feature-tours/unified-dashboard.js` tour in development. Edit `version` to something above the current site kit version. This tour will have missing target on last step by default, to make it work adjust `googlesitekit-widget--searchFunnel` target to be `googlesitekit-widget--searchFunnelGA4`. Middle step target can be edited to something non-existing to confirm close/dismiss behaviour when other than last step has missing target

### Test Coverage

* Update `assets/js/components/TourTooltips.test.js`
* Fix any failing tests

## QA Brief

*

## Changelog entry

*

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.