Simplify tour.js

I really like this patch. I after having others take it for a test I think your right; we should keep the tips in the markup. I have gone through both a manual test and code review.

Keep tips that are not bound to an id or data attribute. Because with something like Tour UI people will add those kind of tips and we need to show them otherwise confusion ensue. That's what happened to me anyway.

I have no idea what this means.

+++ b/core/modules/tour/js/tour.jsundefined
@@ -3,7 +3,7 @@
-(function ($, Backbone, Drupal, document) {

We use document in the code, so why remove it?

+++ b/core/modules/tour/js/tour.jsundefined
@@ -52,17 +53,17 @@ Drupal.behaviors.tour = {
 
-Drupal.tour = Drupal.tour || { models: {}, views: {}};
+var tour = { models: {}, views: {} };

Why these changes?

+++ b/core/modules/tour/js/tour.jsundefined
@@ -141,64 +142,11 @@ Drupal.tour.views.ToggleTourView = Backbone.View.extend({
-  /**
-   * Removes tour items for elements that don't exist.
-   *
-   * @param jQuery $tour
-   *   A jQuery element pointing to a <ol> containing tour items.
-   * @param jQuery $document
-   *   A jQuery element pointing to the document within which the elements
-   *   should be sought.
-   *
-   * @see _getDocument()
-   */
-  _removeIrrelevantTourItems: function ($tour, $document) {
-    var removals = false;
-    $tour
-      .find('li')
-      .each(function () {
-        var $this = $(this);
-        var itemId = $this.attr('data-id');
-        var itemClass = $this.attr('data-class');
-        if ($document.find('#' + itemId + ', .' + itemClass).length === 0) {
-          removals = true;
-          $this.remove();
-        }
-      });
-
-    // If there were removals, we'll have to do some clean-up.
-    if (removals) {
-      var total = $tour.find('li').length;
-      $tour
-        .find('li')
-        // Rebuild the progress data.
-        .each(function (index) {
-          var progress = Drupal.t('!tour_item of !total', { '!tour_item': index + 1, '!total': total });
-          $(this).find('.tour-progress').text(progress);
-        })
-        // Update the last item to have "End tour" as the button.
-        .last()
-        .attr('data-text', Drupal.t('End tour'));

Doing this breaks things. It will show tour items for element that don't exist.

Look at e.g. tour.tour.views-ui.yml. The tips are path-based. But not all of the tips are necessarily present (e.g. the "Search" tip on the front page shouldn't show up if the site is configured to not have a Search block).

Of course, I don't like that very much either, ideally the server side would only send tips that *are* relevant to the current page.

I believe nod_ is referencing that if we hide the markup of the tips that don't match a selector on the page then "tip creators" will be worried that there tips didn't work. I ran my views ui tour with a few broken selectors and they don't break the tour.

Okay :)

As I said in chat to nod_: I'd *love* to get rid of _removeIrrelevantTourItems(). It was necessary in the early Tour days, if it's not anymore: great! :)

Awesome,

As long as we can still filter down the tips on the page as per this: http://drupal.org/node/1942576

I use a query of ?tour=1&tips=tip-example and it filters down the available tips on the page to the ones with the "tips" corresponding class.

Hmm. I think we are going to have to keep that portion of code guys. With the use of wildcards in the URL a tour could span over multiple pages and we would only want the tip to appear on the page that it's element is available on.