Added container tour option, instead of always using document.body

[Benjamin-Dobell]

Closes #206

Alternate implementation of #197, as the most recent commits (those concerning position) seem to result in incorrect bubble positioning (wrong coordinate system). This implementation also supports specifying the container opt as either a string or a DOM element i.e. it functions the same as a step target.

[Benjamin-Dobell]

Hmm, so after I go claiming the positioning is correct in this pull request it fails a positioning test. Whoops. Investigating...

[Benjamin-Dobell]

There we go! All sorted.

Basically we can't reliably use getBoundingClientRect on the container element as it returns the rendered bounding box of the container's current content, if the container is not absolutely positioned (kind of crazy really). Meaning if all the children have margins (like in the test), the returned bounding box will encompass the margin-offset content, rather than the containers actual (left, top) viewport coordinates.

As such, I'm now resetting the bubble and then getting its bounding client rect, which will be the correct basis for the container's coordinate system. I've also added a fixedContainer option, which basically does the same job as a step's fixedTarget option i.e. Let's Hopscotch know that it shouldn't factor in scrolling offsets into its positioning when the container if fixed.

[suhmantha1]

@Benjamin-Dobell What's the status on getting this merged in? I'm looking to use this functionality for an overlay container

[levib]

@Benjamin-Dobell any luck here?

[Benjamin-Dobell]

@suhmantha1 @levib I think you'll want to follow up with @zimmi88 as to why this wasn't merged (or closed). It conflicts now, but didn't at the time. I've been using this in production for 12 months without issue, but @zimmi88 may not be happy with the implementation.

[suhmantha1]

@Benjamin-Dobell I've replaced hopscotch.js with this PR version, but am getting the error Bubble rendering failed - template "bubble_default" is not a function.. How does your tour object look? Are you still starting tour like hopscotch.startTour(tourObject)?

[Benjamin-Dobell]

I've just fixed a bug with the auto-scroll functionality and rebased on master so that this will merge cleanly.

The tests for this pull request (and pretty much every other PR) are failing because Hopscotch isn't using a lockfile - so the CI pulls down the wrong version dependencies. If you're looking to build/test this PR I'd suggest checking out https://github.com/linkedin/hopscotch/pull/360 then cherry-picking this commit on top.

@suhmantha1 Yes, I'm still starting my tour with hopscotch.startTour(tourObject), tourObject looks something like:

{
  id: 'some-id',
  showPrevButton: true,
  bubbleWidth: bubbleWidth,
  steps: [
      {
          title: 'Hiya.',
          content: "Blah blah blah",
          target: someElement,
          placement: 'right',
          arrowOffset: bubbleWidth - 180,
          onNext: doSomeStuff
      },
      /* ... more steps */
  ],
  container: someDomElement
}

[dcantatore]

@linkedin Is this package dead?

[andi23rosca]

For anyone who doesn't want to bother with cloning the repo and going through the whole process Benjamin-Dobell recommended, i did that myself and attached a txt file containing the built code.

index.txt

[vasvir]

I just checked this PR and it works as advertised. I am able to show tours even when my main widget goes fullscreen. I had to manually apply the diff and I took the liberty of some minor edits (minimize diff, handle IE branch).

I am attaching the resulting patch and the resulting hopscotch.js and min.js

In my opinion the maintainer has to accept this pull request with minor modifications.

hopscotch.min.js.txt hopscotch.js.txt 313v2.diff.txt