-
Notifications
You must be signed in to change notification settings - Fork 664
Added container tour option, instead of always using document.body #313
base: master
Are you sure you want to change the base?
Conversation
Hmm, so after I go claiming the positioning is correct in this pull request it fails a positioning test. Whoops. Investigating... |
0d5a54d
to
7959835
Compare
There we go! All sorted. Basically we can't reliably use 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 |
@Benjamin-Dobell What's the status on getting this merged in? I'm looking to use this functionality for an overlay container |
@Benjamin-Dobell any luck here? |
@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. |
@Benjamin-Dobell I've replaced |
04ac385
to
2f81e4f
Compare
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 #360 then cherry-picking this commit on top. @suhmantha1 Yes, I'm still starting my tour with
|
2f81e4f
to
e9c9f7f
Compare
@linkedin Is this package dead? |
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. |
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. |
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 steptarget
.