Updated: Comment #N

Problem/Motivation

Drupal 8 has the new feature tour which allows to add nice popups on a page to explain the page elements(i.e. another type of help module). More details about tour at https://drupal.org/taxonomy/term/41102

Proposed resolution

Create a example module with all possible elements of drupal

Remaining tasks

Issue patch
Review/testing

CommentFileSizeAuthor
#5 2147349_5.patch11.46 KBMile23
#4 tour_example-2147349-4.patch9.17 KBbryanbraun
#4 interdiff-2147349-1-4.txt10.27 KBbryanbraun
#1 tour_example-2147349-1.patch7.14 KBbryanbraun
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bryanbraun’s picture

bryanbraun CreditAttribution: bryanbraun commented
Status: Active » Needs review
FileSize
tour_example-2147349-1.patch7.14 KB

I've attached a patch for including a module documenting a basic tour. Please review and provide feedback when possible. Thanks!

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented

Good start, here are few comments at the first level of review:

  1. +++ b/tour_example/config/tour.tour.tour-example.yml
@@ -0,0 +1,87 @@
\ No newline at end of file

+++ b/tour_example/lib/Drupal/tour_example/Controller/TourExampleController.php
@@ -0,0 +1,29 @@
\ No newline at end of file

+++ b/tour_example/tour_example.info.yml
@@ -0,0 +1,8 @@
\ No newline at end of file

+++ b/tour_example/tour_example.module
@@ -0,0 +1,18 @@
\ No newline at end of file

+++ b/tour_example/tour_example.routing.yml
@@ -0,0 +1,6 @@
\ No newline at end of file

    No new line at the end 

  2. +++ b/tour_example/tour_example.module
@@ -0,0 +1,18 @@
+function tour_example_menu() {

    Guess, we don't need hook_menu 

  3. +++ b/tour_example/lib/Drupal/tour_example/Controller/TourExampleController.php
@@ -0,0 +1,29 @@
+      '#markup' => '<div style="float: left; width: 25%;" id="tour-id-1">1</div><div style="float: left; width: 25%;" id="tour-id-2">2</div><div style="float: left; width: 25%;" id="tour-id-3">3</div><div style="float: left; width: 25%;" id="tour-id-4">4</div>',

    Can we have few more different elements of a page and how can tour provide options to navigate from one to other.

  4. Can we have a README or place to explain what is tour is all about?
Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Needs work

Good work, bryanbraun.

In addition to the coding standards stuff about trailing whitespaces and 80-char lines...

  1. +++ b/tour_example/config/tour.tour.tour-example.yml
@@ -0,0 +1,87 @@
+paths:
+  - 'admin/tour_example'

+++ b/tour_example/tour_example.module
@@ -0,0 +1,18 @@
+  $items['admin/tour_example'] = array(

+++ b/tour_example/tour_example.routing.yml
@@ -0,0 +1,6 @@
+  path: '/admin/tour_example'

    Please have this content at /examples/tour_example.

    That way it's completely visible after the module has been enabled.

It might also be cool to define some tours for other examples, though that's a bit out of scope here.

Also: @vijaycs85: We do need hook_menu(), because Drupal 8 now makes a distinction between a route and a menu path. https://api.drupal.org/api/drupal/core%21includes%21menu.inc/group/menu/8

bryanbraun’s picture

bryanbraun CreditAttribution: bryanbraun commented
Status: Needs work » Needs review
FileSize
interdiff-2147349-1-4.txt10.27 KB
tour_example-2147349-4.patch9.17 KB

Thanks for the feedback! I've made all the codesniffer fixes, added a README document, made a few tweaks to the tour, and changed the example URL. I've kept the new URL in the admin theme, since the tour icon is only available in the toolbar, which isn't visible to non-authenticated users. The primary use case for Tour is for CMS users anyway, not site visitors.

I rerolled the patch to include these changes, but also included an interdiff.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
FileSize
2147349_5.patch11.46 KB

Thanks, bryanbraun.

I was curious how to go about writing tests for tours, so I did some work on this and ran with it.

Any criticism welcome, obviously. :-)

Status: Needs review » Needs work

The last submitted patch, 5: 2147349_5.patch, failed testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs work » Needs review

5: 2147349_5.patch queued for re-testing.

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Component: Other » Tour Example

Committed: http://drupalcode.org/project/examples.git/commitdiff/f01139a9ca43d5cf1d...

Thanks, folks!

Mile23’s picture

Mile23
Primary language English
Location Seattle, WA
CreditAttribution: Mile23 commented
Status: Needs review » Fixed
vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 commented

Thanks to @bryanbraun and @Mile23 for the great work to get this in. Reg #2.2 - yeah @Mile23 is right. may be we are not there yet (i.e. to get hook_menu() out).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.