On one-page applications, depending on the context in which it's set up, pageguide may be run multiple times on the same page. This can cause major issues with conflicting event handlers, multiple ready() timers, repeated (or lost) DOM elements, etc.

Ideally, pageguide should always init() in a clean state, without having to worry about conflicting with existing handlers or elements. I already added a bit of support for this in 6841ead, removing any existing #tlyPageGuideWrapper elements, but it really needs to be taken a step further:

• [x] Before creating any DOM elements, check for previously created ones and remove them
• [x] Unbind any outstanding handlers before re-binding them
• [x] Add a destroy method
• [ ] Consider storing the pageguide object in the global scope inside tl.pg, so the object itself can be deleted safely before creating a new one*
• [x] Handle new pageguide-related elements (#tlyPageGuide, #tlyPageGuideWelcome) gracefully -- persist them if necessary, but if new ones are detected on the page, use those instead

I have done significant work on this in Traceview's copy of pageguide, which I can port over to this repo.

* Is there ever a case where multiple pageguide objects on a single page are desirable? I don't think there is, but am happy to accommodate if anyone has a good argument for allowing it.

This PR adds some primarily functional tests to pageguide, using the markup within the example page. These are certainly not exhaustive but they cover most of its basic use cases (opening/closing, navigating through steps, interacting with the welcome message, etc.). Due to pageguide's asynchronous nature, I had to write a few helper functions to delay test execution, but they still resemble basic QUnit tests for the most part.

I wanted to hold off on writing too many small, specific unit tests for now, since I intend to overhaul pageguide significantly in fixing #28. However, most of this work will be done under the hood, so it will still behave like the pageguide we all know and love (thus, the functional tests, to facilitate further development).

In addition to the new tests, it also adds a few things to pageguide.js:

• Wrap entire body in closure and use $ for jQuery
• Add ready_callback to the tl.pg.init options, allowing a callback to run on pg.ready(). This is primarily useful for running async tests, but could also help users trigger other actions once pageguide is ready.
• Define markup for the guide and toggle within contiguous blocks before appending to the DOM. This makes this markup much easier to read, understand, and edit.
• Add tl.pg.destroy() method, to remove all pageguide-associated DOM elements.
• Automatically wrap #tlyPageGuide step text within container divs to cut down on the amount of markup required for the user to include.
• Class the ins elements appended for step indices for easy reference and removal
• Replace all contextual selections with find(), for performance reasons

Hi,

I use closure compiler to minify my js files. When compressing pageguide, i get some errors because you seems to make use of reserved js words, which is against the standard.

A simple variable rename should fix it.

Here are the involved lines:

line 251 the "str" word : tl.pg.hashCode = function (str) line 256 also "str" word: for (i = 0; i < str.length; i++) line 257 "str": char = str.charCodeAt(i);

Thanks !

cc: @arizzitano

While doing some experimentation with pageguide, I noticed the following in the tl.pg.init function:

https://github.com/tracelytics/pageguide/blob/master/js/pageguide.js#L127-L134

We are displaying the welcome modal before we bind events. Since the event handlers don't get setup until the callback to pg.ready fires (which can take several seconds), this can result in a user clicking on the welcome modal buttons, without any effect.

Thus, I see 3 possible solutions to this problem:

1. Show welcome modal when pageguide is ready (delayed)

pg.ready(function() {
    pg.setup_handlers();
    pg.$base.children(".tlypageguide_toggle").animate({ "right": "-120px" }, 250);

    if (pg.preferences.show_welcome) {
        pg.pop_welcome();
    }
});

2. Show welcome modal immediately

pg.ready(function() {
    pg.$base.children(".tlypageguide_toggle").animate({ "right": "-120px" }, 250);
});

pg.setup_handlers();
if (pg.preferences.show_welcome) {
    pg.pop_welcome();
}

This approach sets up event binding before pageguide is ready, so I'm not sure if this will break anything.

3. Isolate welcome modal event handlers

// New method for setting up only the event handlers for the welcome modal.
tl.pg.PageGuide.prototype.setup_welcome_handlers = function () {
    ...
}

pg.ready(function() {
    pg.setup_handlers();
    pg.$base.children(".tlypageguide_toggle").animate({ "right": "-120px" }, 250);
});

if (pg.preferences.show_welcome) {
    pg.setup_welcome_handlers();
    pg.pop_welcome();
}

Assuming that the pg.setup_handlers(); needs to be called in the pg.ready callback, I think option 3 is the best choice.

Thoughts?

In my case, I have kind of accordion menus, I would like a trigger a bit like your tracking trigger but called when a new doc string is displayed with "data-tourtarget" value as arg. (eventually from the closed doc too).

This would allow me to open, close correspondents menus and to display/hide panels accordingly.

I sound easy to implement and may be helpfull for other persons than me, ask me if you want me to do it or if you prefer do it yourself/do not pull that kind of stuff.

This patch improve the guide's behaviour with fixed elements. If the target element has the data attribute 'position' assigned to 'fixed', the position of the arrow is calculated considering the top of the window.

Hey @dkuebric do you mind removing the AppNeta logo from the pageguide site ? We've gotten inquiries on our appneta.com live chat about whether or not we are still maintaining it. You could also remove AppNeta from the license.

Thanks!

I updated my addSteps function to support fixed elements

  tl.pg.PageGuide.prototype.addSteps = function () {
    var self = this;
    self.$steps.find('li').each(function (i, el) {
      var $el = $(el);
      var tourTarget = $el.data('tourtarget');
      var positionClass = $el.attr('class');
      var style = '';
      if (self.targetData[tourTarget] == null) {
        self.targetData[tourTarget] = {
          targetStyle: {},
          content: $el.html()
        };
        var hashCode = tl.pg.hashCode(tourTarget) + '';
        self.hashTable[hashCode] = tourTarget;

        function isFixed(ele, posType) {
          posType = posType || "fixed";
          if (ele.prop("tagName") === 'HTML') {
            return false;
          }
          return (ele.css("position") !== posType) ? isFixed(ele.offsetParent()): posType;
        }

        if (isFixed($(tourTarget))) {

          style = 'position:fixed;';

        }

        self.$content.append(
          '<div class="tlypageguide_shadow tlypageguide_shadow' + hashCode +
          '" data-selectorhash="' + hashCode + '" style="' + style + '">' +
          '<span class="tlyPageGuideStepIndex ' + positionClass + '"></span>' +
          '</div>'
        );
      }
    });
  };

might be useful to others

magic happens here

function isFixed(ele, posType) {
  posType = posType || "fixed";
  if (ele.prop("tagName") === 'HTML') {
    return false;
  }
  return (ele.css("position") !== posType) ? isFixed(ele.offsetParent()): posType;
}

if (isFixed($(tourTarget))) {

  style = 'position:fixed;';

}

When an element is positioned under the messages overlay at the bottom of the screen, the detection mechanism is flawed and the element will not scroll into view. For example, in my case (using Chrome 42.0.2311.39 beta (64-bit)) in the method tl.pg.isScrolledIntoView the values are :

dvtop 0 dvbtm 1115 eltop 996 elbtm 1008

The messages div is 100px in height, but also has 10px of padding on all sides. The check in the method determines if the bottom of the element is below the top of the viewport, and if the top of the element is above 100px from the bottom of the viewporrt.

There are 2 problems with the check.

1. It should check if top the element is below the top of the viewport.
2. It should check if the bottom of the element is above the top of the #tlyPageGuideMessages div with padding.

Below is a diff against 1.3.1 which solves the issue. (Forgive my github knowledge - if there is a more appropriate way to provide the diff, let me know and I will try to accomodate.) This change reorders the animation of the messages box with the animation of the element scrolling into view, so that we can make use of .outerHeight() for the collision check.

@@ -266,13 +266,13 @@ tl.pg.interval = {}; * currently scrolled viewport. * elem (string): selector for the element in question **/

• tl.pg.isScrolledIntoView = function(elem) {

• tl.pg.isScrolledIntoView = function(elem, height) { var dvtop = $(window).scrollTop(), dvbtm = dvtop + $(window).height(), eltop = $(elem).offset().top, elbtm = eltop + $(elem).height();

•    return (elbtm >= dvtop) && (eltop <= dvbtm - 100);
  

•    return (eltop >= dvtop) && (elbtm <= dvbtm - height);
  

  };

  /** @@ -533,10 +533,10 @@ tl.pg.interval = {}; height = $(window).height()/2; }

•        if (!tl.pg.isScrolledIntoView($(targetKey))) {
  

•        this.$message.show().animate({'height': height}, 500);
  

•        if (!tl.pg.isScrolledIntoView($(targetKey), this.$message.outerHeight())) {
           $('html,body').animate({scrollTop: target.targetStyle.top - 50}, 500);
       }
  

•        this.$message.show().animate({'height': height}, 500);
       this.roll_number(this.$message.find('span'), target.index);
   }
  

  };

It would be great if the requirements were included in the readme file (jquery & jquery ui, etc...)

I am trying to use this on a drupal 7 install and it doesn't seem to be working I think it might be a jquery version issue...

Currently, when you switch to a new item in the pageguide (using the previous and next buttons, or by jumping to an item by clicking the element on the host page), the number first changes to the updated value and then rolls over.

The expected behavior is for the previous item number to roll out of view, and have the new item roll into view.

I have an app where the content is being written into a div and the elements I want to highlight are in that div. PageGuide writes everything into the body. How can I have it write it into the div with the content so it will all scroll well and such?

Hello, I cloned the project and followed the instructions, but when I do a grunt server I get the following error:

$ grunt server Running "clean:dist" (clean) task

Running "jshint:gruntfile" (jshint) task

1 file lint free.

Running "jshint:src" (jshint) task

1 file lint free.

Running "jshint:test" (jshint) task

1 file lint free.

Running "qunit:files" (qunit) task Testing js/tests/index.html .........................OK Fatal error: Callback must be a function. Received undefined

Here are the versions of node and grunt I'm using: $ node -v v14.16.0

$ grunt -version grunt-cli v1.4.2 grunt v0.4.5

If you have any clickable content underneath where the page guide expands, you won't get a pointer cursor and you can't click it.

The tlyPageGuideToggles element needs a pointer-events: none style, and its children of class tlypageguide_toggle need it set back to auto.

It'd be nice to have a step that doesn't point to any particular element, for including support URLs and general comments.

<ul ...>
  <li data-tourtarget="#mylastelement">
    <p>Finally, this is where ...</p>
  </li>
  <li>
    <p>That wraps it up. Having any trouble? Contact us at ...</p>
  </li>
</ul>

You can technically do this already by pointing to the body, but it puts an overlay over everything, which isn't desirable.

tl.pg.hashUrl = function () { return tl.pg.hashCode(window.location.href); };

Should probably be

tl.pg.hashUrl = function () { return tl.pg.hashCode(window.location.hostname + window.location.pathname); };