Allow Tours to be taken by users that cannot access the Toolbar (e.g. anonymous users)

A good idea could be to provide a block with the button that site builders can place wherever they want.

We are interested in providing a Tour for anonymous users in our project so I am going to start on this.

I'm retitling this and transforming it into a feature request since solving it with a block feels more like a feature addition than a bugfix. This means that this will be for 8.5.x at the earliest, but it is very likely that the patches will also work on 8.4.x.

Starting off by creating a simple block that now shows the button on every page. It is already able to launch the tours if we are on a tour-enabled route.

Next step will be to only show the block on tour-enabled routes.

Interesting, so it actually already works as it should, since the showing / hiding of the button is handled in JS. I just added an access control here so the button will not be loaded if the user doesn't have permission to view tours.

This is going pretty smoothly. Next step is providing a test.

Some small improvements:

• Correctly apply the 'hidden' CSS class to the block wrapper so the block doesn't "flash" when a page with no tour is loaded.
• Since the block will be present on any page and be hidden using JS we don't need to cache by route.

Added a test. I opted for a JS test since none of the tour functionality was actually tested in a JS enabled browser. Now this also asserts that the tour tips actually become visible when the tour button is clicked, not just that they are present in the HTML.

Extended the test a bit to make sure the tour button doesn't show up on pages that do not offer a tour.

Tested it and works as designed! The tour button is properly placed.
Patch seems fine to me.
Moving to RTBC.

1. +++ b/core/modules/tour/src/Plugin/Block/TourButtonBlock.php
   @@ -0,0 +1,54 @@
   +        '#value' => t('Tour'),
   

   Can't we use $this->t( instead of a regular t here?

2. +++ b/core/modules/tour/tests/src/FunctionalJavascript/TourButtonBlockTest.php
   @@ -0,0 +1,82 @@
   +  /**
   +   * Modules to enable.
   +   *
   +   * @var array
   +   */
   +  public static $modules = ['block', 'tour', 'tour_test'];
   

   This can be {@inheritdoc} instead of the complete doc.

@borisson_, we can definitely use $this->t() in here given this is a custom block plugin. Also, since we not overriding the doc comment here in any specific way the docblock in the base class BrowserTestBase makes more sense.

Uploading an updated patch here.

Awesome, this was already rtbc in #14 and the nitpicks I found were fixed. so back to rtbc it is.

I think this is a great idea. (Providing the toolbar to anonymous users has funny results indeed, which is a separate bug.)

I tested this patch with the following steps:

1. Grant permission to access tours and the views UI (lol) to the authenticated user role.
2. Place the block in the sidebar.
3. Log in as a test user and visit admin/structure/views.
4. Click on a view to access it.
5. Click the tour button in the sidebar.

A few improvements I think we should make:

1. I think we should make it not display the block title by default because it looks kind of silly:
   

   This can be done with:

     /**                                                                           
      * {@inheritdoc}                                                              
      */
     public function defaultConfiguration() {
       return ['label_display' => FALSE];
     }
   

2. I also think it'd be good to provide some styling for the button that's similar to what's available in the toolbar, with the question icon.
3. A side effect of exposing tours outside admin pages is that they're not styled with the admin theme. They look pretty icky in Bartik:
   

   That might be a separate issue scope, not sure. It might be a followup. But I think we should at least provide a border for the tips in Bartik in this patch so that they're not totally confusing and broken-looking (since this patch will remove the assumption that Tour pages only show up on administrative routes).

Thanks!

I tried my hand at some basic styling. My frontend chops are very poor so I just shamelessly stole the CSS from the Toolbar module :D

This is how the tour button now looks in Bartik:

And the balloons:

We can indeed further refine the styling in a followup.

1. +++ b/core/modules/tour/src/Plugin/Block/TourButtonBlock.php
   @@ -0,0 +1,63 @@
   +        '#tag' => 'button',
   

   Maybe we can add a class in here, so we can target the element through that, instead of using the element type directly.
   Maybe we can even use ".button", so we inherit the button styles present already.

2. +++ b/core/themes/stable/css/tour/tour.module.css
   @@ -140,3 +140,45 @@
   +  background-image: url(../../images/core/icons/787878/questionmark-disc.svg);
   +  background-attachment: scroll;
   +  background-color: transparent;
   +  background-position: center center;
   +  background-repeat: no-repeat;
   

   This can be squeezed into a single line declaration.

3. I noticed that the "nubs" (the little arrows that point to an element) are not styled and thus not visible for the Bartik theme. Do you think we should handle them later on, or put some default style also there?

Addressed the remarks:

1. Good suggestion, fixed.
2. I tried putting it on one line but I struggled with the background-size. I read the documentation of the CSS3 background property but it is not very comprehensive on the sizing part. I tried a few things like "center/100% center/auto" but in the end the only thing that worked reliably is splitting it off on a second line. The other properties were no problem.
3. I think that would be better in a dedicated issue in the Bartik issue queue. I created a followup for it so it won't be forgotten: #2935334: Theme tour help balloons and button.

I would move ahead with it but I think the #2935334: Theme tour help balloons should be merged in first. An admin can already set batrik as the admin theme but it's not very common. This change can make it easy for a lot of tours to be created.

+++ b/core/themes/bartik/bartik.libraries.yml
@@ -46,6 +46,7 @@ global-styling:
+      css/components/tour.css: {}

+++ b/core/themes/bartik/css/components/tour.css
@@ -0,0 +1,8 @@
+/**
+ * @file
+ * Tour module overrides for Bartik.
+ */
+
+.joyride-tip-guide {
+  border: 1px solid #ccc;
+}

this feels out of scope, but flagging for the front-end framework managers to review css

also, can we can get an IS update to indicate the solution - uses a block

@larowlan, I think it needs to be in scope because of the visual bug; see #18. I was actually thinking we should also maybe additionally put a border by default in the module's CSS, so that other themes don't have tour tips bleeding into the background in a hard-to-read way.

Hm, patch is also adding CSS to Stable which I think might be considered a BC break. (Safe places to put it might be the module or the internal themes.) Not sure what the policy is on adding classes to Stable to new UI elements. Edit: Also, at least, I think the styles should go in Classy and not stable?

Stable is supposed ship with a minimum required CSS (only functional CSS). If we'd want to add these styles to the base themes, it should be inside Classy as pointed by @xjm on #27.

I would suggest that we leave all of the design decisions like this one for the themes themselves, meaning that we only apply this design in Seven and Bartik. The styles the latest patch adds for the button in Stable could be used as a base for the Seven and Bartik styles.

I agree with #18 that button with a text "Tour" can be confusing. Rather than adding the question mark, could we solve this by changing the wording for something more descriptive such as "Start a tour"?

Anyway, I reviewed the CSS code and by itself, it looks fine to me. Since there's some design involved with this, It would be great to get a UX team, and potentially later a product manager to review the design.

also, can we can get an IS update to indicate the solution - uses a block

Updated the issue summary.

I agree with #18 that button with a text "Tour" can be confusing. Rather than adding the question mark, could we solve this by changing the wording for something more descriptive such as "Start a tour"?

Let's indeed change this to "Start a tour" and also make the text configurable in the block options.

Small clarification of @lauriii's comment in #28. My understanding is that he suggests to NOT move the basic styling into the 'classy' theme, but rather move it directly into Seven and Bartik.

In general, basic styling for cosmetic purposes should be in Classy and not in Stable, so that it can be inherited by core themes and custom themes built on top of Classy. But this is a special case. These balloons can be implemented in very different ways, so any basic theming being present in Classy will just get in the way of custom themes and would need to be overridden.

So this means we will have duplication of the styling between Seven and Bartik, but that's an acceptable compromise.

What about "Take a tour"? I don't know but it seems to me better than the "Start a tour" especially with the question mark in front.

"Take a tour" as well sounds good for me.

Also, regarding the basic styling for batrik theme, a bit further refinement is needed. I tried the honeypot tour and the following two things happened:

1. When the tour box is below the target element, there is a small overlap as shown in the attached picture. These are only a few pixels though.
2. When the tour box is above the element, and the window repositions, and the tour is high enough, a bit of text moves above the screen. This behaviour occurs also in the seven theme. The full window is not shown but the window is repositioned for the main content of the tour window.

Also, sometimes it is not clear the element that the tour targets.

All these need to be fixed as for basic functionality.

Changes included:

• Moved styling for the button to the batrik and seven themes respectively.
• Change the title of the button from "Tour" to "Take a tour".
• Style the nub (little arrow) similarly to the seven theme styling but use the colour #ccc which is used in the modal border. This fixes the overlap issue. I did not improvise there at all because of UX design decisions needed.
• The "Next"/"End" button is using same styling as the "Take a tour" button.
• Renamed the tour.css to tour.theme.css to have the same naming as the seven theme's corresponding file.

Hope I did not forget anything.
The image contains the styled nub.

Fix a minor remark.

@clemens.tolboom your patch failed because you did not branch off my latest one and it was one with a leftover in the tests (sorry for that, I hid that file).

Thanks for the background set.
The differences from diff core/themes/bartik/css/components/tour.theme.css core/themes/seven/css/components/tour.theme.css are simple theming. As I mentioned above and as @laurii and @xjm noted, decisions on the design should not be picked up without someone from the design team have a say. So what I did is I tried to re-use whatever already exists so that nothing seems as new design decisions.

This means that the colours should also match the respective theme colours. So the arrow is filled with the colour of the border of the tour box which in the specific case is #ccc.

For example, I do not mind that you added the background for the case that is a generic text. I did not forget it, it just didn't exist before (it did in the Seven theme only) so I didn't want to mess with the FE implementation of the thing that much. But if this is accepted, indeed it looks nicer.

In any case, please update your patch to start off the latest patch (2069073-38.patch)

Following up @clemens.tolboom remarks.

It looks like we should have a usability review or what exactly the block should be displayed. After that we should probably have Lauri look at it again too. Thanks!

+++ b/core/modules/tour/js/tour.es6.js
@@ -28,7 +28,7 @@
+          el: $(context).find('#toolbar-tab-tour, .block-tour-button-block'),

We should add a data attribute or js- prefixed class to ensure that developers are aware that the element has JS attached to it.

@lauriii good idea, can you suggest a good class or data attribute to use?

@pfrenssen How about js-tour-start-button?

Great suggestion!

+++ b/core/modules/tour/tests/src/FunctionalJavascript/TourButtonBlockTest.php
@@ -0,0 +1,81 @@
+    $elements = $this->cssSelect('.block-tour-button-block button');

We could use the js prefixed selector added in the most recent version of the patch here as well 🙌 Other than that everything looks good for me ✨

Here is the interdiff and patch based on the comment #57.

This should fix the test failure.

I'm wondering if this actually still need a usability review? The block only shows the toolbar button and it can be placed anywhere the site builder wants it.

Straight reroll of #60.

Fix the test.

No, this needs no usability review. This is a block that shows the same button as in the toolbar.

I only did a reroll and a very small change to use WebDriverTestBase instead of the deprecated one. We tested this block and we're already running it for anonymous users in our project, in production. All the previous concerns and remarks were addressed. So, RTBC.

I do still think that we should get someone design oriented to review the new components this issue introduces. This doesn't use the pre-existing button styles, but rather introduces something new. If this would be just about using pre-existing components we have, we wouldn't necessarily need a review.

+++ b/core/themes/bartik/css/components/tour.theme.css
@@ -56,48 +49,40 @@
+.block-tour-button-block .tour-button {
...
+[dir="rtl"] .block-tour-button-block .tour-button {
...
+.block-tour-button-block .tour-button:before {
...
+[dir="rtl"] .block-tour-button-block .tour-button:before {
...
+.block-tour-button-block .tour-button:active:before,

+++ b/core/themes/seven/css/components/tour.theme.css
@@ -101,3 +101,41 @@
+.block-tour-button-block .tour-button {
...
+[dir="rtl"] .block-tour-button-block .tour-button {
...
+.block-tour-button-block .tour-button:before {
...
+[dir="rtl"] .block-tour-button-block .tour-button:before {
...
+.block-tour-button-block .tour-button:active:before,
+.block-tour-button-block .tour-button.is-active:before {

We should remove the block class name from the selectors since it should be possible to place the tour button outside the block.

I split the triggering element selector in #3012027: Decouple tour triggering from the toolbar.

Hello,

I've trained to apply this patch at 8.6.3 version of Drupal Core and it fails. In 8.7-dev version it works properly.

I've made a patch with the Tour Block which applies on 8.6.3 version.

Please, check it. It has no tests nor CSS.

Thanks @maboresev for rolling this against 8.6.3. Unfortunately since this is a feature request this won't be accepted for Drupal 8.6.x which only receives bug fixes. New features are developed against the next release of Drupal core, so I will set this back to 8.7.x.

The current patch is the one from #64.

The previous patch submitted on #64 not working with 8.8.x branch.

The attached patch is the same but corrected with the changes on the original file.

I can't attach the interdiff file because it fails on creation because both patches are of different versions of the original files.

The patch attached on #64 not work with 8.7.1, for this reason I hide it.

Greetings.

Setting back to Needs Review & Triggering bots.

Patch in #74 works for me, thanks.

Applied #74 and works fine!
You need to clear your browser cache, otherwise the required java is not loaded

Thanks for the patch :)

Work for me in Drupal 8.9.1, but the tests don't complete.

Rerolling patch for 9.1.x!

Fixing test cases!

Patch #83 looks good to me.

Set to RTBC.

Actually I think we need Usability review before commit it. Am I right?
I'll set back to needs review

Is there any update? When it will be available for drupal core update? Dont want to aply the patch for the core modules.

Would be nice if we would be able to configure the button label?

Any luck with 8.9.10? Seems like none fo the patches work for this version

Same here, been waiting for this to be included in core for a while now.

The patch in #83 applied to 8.9.13 for me with a little offset. This patch really helps with doing custom tours since we don't have to rely on the menu link and can use the block.

In Drupal 9.2+ there are some pretty significant changes to Tours in core. The jQuery Joyride library has been deprecated and replaced with Shepherd JS.

This issue may be effected.

Deprecate and replace jQuery Joyride (for tours)
Issue: https://www.drupal.org/project/drupal/issues/3051766

Joyride library deprecated
Change Notice: https://www.drupal.org/node/3195234

Tour schema properties 'location' and 'attributes' deprecated
Change Notice: https://www.drupal.org/node/3204093

FYI - Patch & functionality works perfectly on Drupal 9.2.4

edit 1: also works on 9.2.5
edit 2: also works on 9.2.6

Ignore this patch, it was not correct.

ok, trying this again.

Attaching a reroll of the patch in #83 that removes the styling, and adds the .button class instead of the .tour-button class.

• Most of the styling was for the previous joyride library.
• The "Take a tour" button label change removed the need for the ? icon IMHO.
• Using the theme's default button styles may remove the need for a UX review (?)
• Shepherd and Tour's default styles are brought in using the tour/tour-styling library.

The testbot shows that there are some coding standards issues. That is what "Custom commands failed" means, and it also means that other tests were not run. One of these days, the testbot will set issues to NW in this case, but for now I will do that manually.

Resolved CS issues

Updated the patch to add the button class which I thought I had added in #98.

Per discussion in Drupal Usability Meeting 2021-12-03, using the .button class and not adding additional CSS negates the need for a UX review/approval.

It was suggested to check with A11Y team, which I will queue.

Removing UX tags, adding A11Y review tag.

This sounds interesting wonder if the accessibility team had a chance to take a look.

Should this be closed for #3012027: Decouple tour triggering from the toolbar?

This extension is being deprecated, see #3336033: [Meta] Tasks to deprecate Tour module. It will be removed from core and moved to a contrib project, #3376099: [11.x] [Meta] Tasks to remove Tour.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.