Updated based on comment #8

Tour allows you to automatically start a tour on a given path, and link to a part of that tour, however this assumes that Tour's only live on specific paths, or that tours have only one condition (autostart). It would be very useful to be able to specify which tour should be run, so that tours can be triggered on different paths, or only loaded when requested. This opens up conditional tours, multi-step tours, multipage tours, and other niceties.

Problem/Motivation

Currently, to get a tour to load on a page, you must specify its path. That basically means we have ONE way of loading a tour onto a page, and that is path matching. You could consider this a 'default' tour behaviour. This creates several fixed outcomes:

  • All tours get loaded at once and added together (which is frankly confusing)
  • The order of tours can't be altered (which is worse)
  • The "tip" selector can be provided, but all this does is select a different starting point on the existing tours. If there are multiple tours on the page, this effectively starts you in the middle of the tour somewhere, which is also a bit confusing.

Basically, there is no way to selectively enable a tour.

Proposed resolution

tour_preprocess_page() will include an additional check on the 'tour-id' query parameter, which can specify one or more tour IDs to load. These will be loaded INSTEAD of any tours on that path.

Use cases

This patch enables you to selectively trigger a tour, and ONLY THAT TOUR, on any path.

Why might you want to do that? Well, there are a number of reasons:

1) You want to help a user with a specific piece of complex functionality, wherever that functionality exists

For example, to allow the Toolbar Tour to be triggered ON ANY PAGE, and not just on the homepage. This allows the user to take the tour whenever is convenient (keeping in mind that we don't currently have a built in way to see what tours are available, though a block could do this, for example).

2) You want to make branching tours.

#1920468: Write a tour for the first page after install showing extend and other things currently proposes a branched tour for the homepage, which allows users to click through to the parts of the tour that they are interested in. This allows the page tour to be a small index of available tours, rather than an unwieldy and massive single tour.

3) You want to load a specific tour when a condition is met that is not path based

For example, you may want to load a tour the first time a user visits a page containing a special block you have provided. This has nothing to do with paths.

What's the use to load tours with no matching path? Their tips will not match elements on the page, right?

Wrong. Whether a path exists has nothing to do with whether the selectors match, its just the way we've chosen to determine what should be visible BY DEFAULT. The selectors may still not be there (and if they aren't the tour isn't offered anyway). It's up to the developer of the tour to ensure that the correct selectors are available when the tour is loaded. This is the same whether its path-based, or triggered.

Because this patch implicitly requires an implementer to create links to the tour's they want, there is actually a GREATER chance that the selectors will be present.

Remaining tasks

This issue.

User interface changes

None

API changes

This issue will introduce a new alter hook, hook_tour_list_alter(), which will run after tours have been selected in tour_prepocess_page(), and give contrib an opportunity to influence which tours are loaded on the page.

#1942576: Tour tips to be able to link to other pages and start tour's automatically.

CommentFileSizeAuthor
#16 drupal8.tour-module.2073891-16.patch4.49 KBxtfer
FAILED: [[SimpleTest]]: [MySQL] 55,210 pass(es), 125 fail(s), and 170 exception(s). View
#2 drupal8.tour-module.2073891-2.patch3.64 KBxtfer
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es). View
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

xtfer’s picture

Issue tags: +Tour

Adding tags

xtfer’s picture

FileSize
3.64 KB
PASSED: [[SimpleTest]]: [MySQL] 58,122 pass(es). View

This patch reworks tour_preprocess_page() so that it will first check if the 'tour-id' is provided, and load that if it is.

I've also refactored slightly to minimise database hits, and moved the section which will eventually be replaced with EFQ into its own function, for ease of refactoring.

xtfer’s picture

Status: Active » Needs review
larowlan’s picture

Status: Needs review » Active
Issue tags: +Needs tests
  1. +++ b/core/modules/tour/tour.module
    @@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
    +    Drupal::moduleHandler()->alter('tour_list', $all_tours, $path, $path_alias);
    

    This looks wrong, $all_tours is not defined

  2. +++ b/core/modules/tour/tour.module
    @@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
    + *   [optional] The path to check tours for. Defaults to the current path.
    ...
    + *   [optional] The path alias. Defaults to the current path alias.
    

    Standard is (Optional)

  3. +++ b/core/modules/tour/tour.module
    @@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
    +  Drupal::moduleHandler()->alter('tour_list', $all_tours, $path, $path_alias);
    

    This duplicates hook_tour_load(), running after the entities are load, it makes more sense running before the entities are loaded, in my opinion. Either way we need to add an entry to tour.api.php

Also, we need a new test for the ?tour-id= functionality, we have tour_test.module for use in tests, there are some tours in there we could test with, we could fetch the path with the tour-id flag and ensure only the markup for those tours is inserted in the dom.

larowlan’s picture

Status: Active » Needs review

crosspost

clemens.tolboom’s picture

What's the use to load tours with no matching path? Their tips will not match elements on the page, right?

A related issue (I need to move to core) is Tour UI #2073321: We need to disable | enable tours

nick_schuch’s picture

clemens.tolboom,

Can you please create an issue so we can work on that there.

xtfer,

Tour tips get provided a class with the tour id assigned. Which allows for ?tour=1&tips=
to load a specific tour.

Can you please provide a use case for when we should use the tour-id parameter over the above? That might make the ticket clearer for review.

xtfer’s picture

Currently, to get a tour to load on a page, you must specify its path. That basically means we have ONE way of loading a tour onto a page, and that is path matching. You could consider this a 'default' tour behaviour. This creates several fixed outcomes:

  • All tours get loaded at once and added together (which is frankly confusing)
  • The order of tours can't be altered (which is worse)
  • The "tip" selector can be provided, but all this does is select a different starting point on the existing tours. If there are multiple tours on the page, this effectively starts you in the middle of the tour somewhere, which is also a bit confusing.

Basically, there is no way to selectively enable a tour.

This patch enables you to selectively trigger a tour, and ONLY THAT TOUR, on any path.

Why might you want to do that? Well, there are a number of reasons:

1) You want to help a user with a specific piece of complex functionality, wherever that functionality exists

For example, to allow the Toolbar Tour to be triggered ON ANY PAGE, and not just on the homepage. This allows the user to take the tour whenever is convenient (keeping in mind that we don't currently have a built in way to see what tours are available, though a block could do this, for example).

2) You want to make branching tours.

#1920468: Write a tour for the first page after install showing extend and other things currently proposes a branched tour for the homepage, which allows users to click through to the parts of the tour that they are interested in. This allows the page tour to be a small index of available tours, rather than an unwieldy and massive single tour.

3) You want to load a specific tour when a condition is met that is not path based

For example, you may want to load a tour the first time a user visits a page containing a special block you have provided. This has nothing to do with paths.

What's the use to load tours with no matching path? Their tips will not match elements on the page, right?

Wrong. Whether a path exists has nothing to do with whether the selectors match, its just the way we've chosen to determine what should be visible BY DEFAULT. The selectors may still not be there (and if they aren't the tour isn't offered anyway). It's up to the developer of the tour to ensure that the correct selectors are available when the tour is loaded. This is the same whether its path-based, or triggered.

Because this patch implicitly requires an implementer to create links to the tour's they want, there is actually a GREATER chance that the selectors will be present.

xtfer’s picture

Status: Needs review » Needs work

+++ b/core/modules/tour/tour.module
@@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
+ Drupal::moduleHandler()->alter('tour_list', $all_tours, $path, $path_alias);
This looks wrong, $all_tours is not defined

Good catch. Refactoring error.

+++ b/core/modules/tour/tour.module
@@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
+ * [optional] The path to check tours for. Defaults to the current path.
...
+ * [optional] The path alias. Defaults to the current path alias.
Standard is (Optional)

I swear it used to be the other way round...

+++ b/core/modules/tour/tour.module
@@ -111,25 +111,76 @@ function tour_preprocess_page(&$variables) {
+ Drupal::moduleHandler()->alter('tour_list', $all_tours, $path, $path_alias);
This duplicates hook_tour_load(), running after the entities are load, it makes more sense running before the entities are loaded, in my opinion. Either way we need to add an entry to tour.api.php

Only because we do an entity_load for this, and not for its other invocation, but you are correct and I will normalise it.

Also, we need a new test for the ?tour-id= functionality, we have tour_test.module for use in tests, there are some tours in there we could test with, we could fetch the path with the tour-id flag and ensure only the markup for those tours is inserted in the dom.

Okay.

larowlan’s picture

FWIW, the tip selector doesn't select the starting point, it filters down the tips to those matching that class. Which I think is what you want to achieve.

Eg given page A with tour X (with three tips) and tour Y (with four tips).

You would have 7 tips for display.

if you used the tip selector, it would only show a subset of that 7. This might be all of tour X, all of tour Y, a single tip, or a different subset, based on the classes given to the tips in each tour via the yaml.

clemens.tolboom’s picture

xtfer’s picture

FWIW, the tip selector doesn't select the starting point, it filters down the tips to those matching that class. Which I think is what you want to achieve.

I think that would have quite a different outcome, and it doesn't help you load tours on any path or based on conditions other than path.

clemens.tolboom’s picture

I just reread the summary and am confused.

Thanks for working on this :)

xtfer’s picture

This issue has some of #1921144: Role based system for tips

There's no cross-over. That is strictly permissions.

We have multipage tour as we can add links to other tours. What is a multistep tour?

Multipage is a form of multistep. Essentially, linking between tours.

What non-coder problem are we trying to solve... If possible I prefer adding a condition attribute for tips and tours instead as Tour Writers (non-coders) need to write tours.

And they still can. Adding a URL with a query parameter in it for a tour-id doesn't change that in any way.

We do have hook_tour_tips_alter used in ie #2017471: Multilingual tour for language section. What adds this issue?

This is run AFTER tips are loaded, which is inefficient. As noted above in #9, we want to be able to get in there before we load any tips.

AFAIK tours are NOT displayed together. That why I partly filed #2074835: Tour: make operations enable | disable available for tours

Load more than one tour on a page, and you get multiple tours strung together, both confusing and unnecessary.

Code is using

 enity_views_multiple() 

so I suspect a bug

There are patch dependencies preventing this from being factored out. This is noted in the code comments.

It would be awesome to have the summary cleaned up and explaining the User goals.

Ive already put 3 use cases in the summary.

clemens.tolboom’s picture

Patch does not apply anymore so needs a re-roll.

In #1921152-57: META: Start providing tour tips for other core modules. I noted we need task oriented tours so this issue and a few others could become handy.

clemens.tolboom’s picture

Issue summary: View changes

Updated summary based on comments

xtfer’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
4.49 KB
FAILED: [[SimpleTest]]: [MySQL] 55,210 pass(es), 125 fail(s), and 170 exception(s). View

Updated patch.

Status: Needs review » Needs work

The last submitted patch, 16: drupal8.tour-module.2073891-16.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.