IGreatlyDislikeJavascript / bootstrap-tourist

Quick and easy way to build your product tours with Bootstrap Popovers for Bootstrap 3 and 4. Based on Bootstrap-Tour, but with many fixes and features added.
62 stars 35 forks source link

Working with element in iFrame #20

Open saviemedia opened 5 years ago

saviemedia commented 5 years ago

Hi, thanks a lot for your Tour.

I've tried to Dynamically determine element by function and I always get this error: Uncaught Error: TOOLTIP: Option "selector" provided type "element" but expected type "(string|boolean)".

Here is what I tried: { element: function() { return $("#myIframe").contents().find("body") }, title: "Help", content: "Check this iFrame" }, Thanks in advance!

thenewbeat commented 5 years ago

You're returning the DOM element, and not the element selector. $("#myIframe").contents().find("body") If you run this in the console you'll see that it returns the HTML element.

The element option for a step is expecting a string: something like:

steps: [
  {
    element: "#myIframe"
  }
]
IGreatlyDislikeJavascript commented 5 years ago

@thenewbeat - Tourist adds support for finding element dynamically by function :-)

@saviemedia - interesting problem, I wonder if there's an issue with the jq object identification. In short, original Tour passes the element string literal around, and I had to make a bunch of changes to allow seamless switch between string literal and function for element.

Are you using bs3 or bs4?

Please can you try creating a tour with a single step that directly returns a DOM element from a function, without using $.find etc? This will let me narrow down whether the problem is the jq code (can't find the element etc), or in Tourist.

In other words, please do this:

In your html: <button id="btnTest" .........>test</button>

In your javascript to set up the tour steps, have just one step that returns the object directly:

steps:
[
    {
        element: function() { return $("#btnTest"); },
        title: "test",
        content: "test content",
    }
]

And in your tour options, please turn on debug for console messages:

var Tour=new Tour({ debug: true, ....other options here... });

This will let us narrow down the problem. Thanks!

saviemedia commented 5 years ago

Thanks for your quick answer! I tried what you suggested to me:

steps: [{ //element: "#selectMatter", element: function() { return $("#selectMatter"); }, title: "Help", content: "This is a test.", }]

When the element is in static mode, it works. When in dynamic mode, I can see the backdrop appearing, but the tooltip won't appear since I get this error:

[ Bootstrap Tour: 'Test' ] Using framework template: bootstrap4 bootstrap-tourist.js:1598 [ Bootstrap Tour: 'Test' ] Extending Bootstrap sanitize options bootstrap-tourist.js:1598 [ Bootstrap Tour: 'Test' ] Scroll into view. ScrollTop: 0. Element offset: 279. Window height: 757. util.js:137 Uncaught Error: TOOLTIP: Option "selector" provided type "element" but expected type "(string|boolean)". at Object.typeCheckConfig (util.js:137) at i.t._getConfig (tooltip.js:632) at i (tooltip.js:129) at new i (popover.js:11) at HTMLSelectElement. (popover.js:158) at Function.each (jquery-3.3.1.min.js:2) at w.fn.init.each (jquery-3.3.1.min.js:2) at w.fn.init.i._jQueryInterface [as popover] (popover.js:149) at Tour._showPopover (bootstrap-tourist.js:1837) at Tour._showPopoverAndOverlay (bootstrap-tourist.js:1703)

Thanks!

IGreatlyDislikeJavascript commented 5 years ago

I am really beginning to hate popper.js!

Ok this was my mistake and I'm not sure how I missed it. Please do this to solve it.

On line 1836 of bootstrap-tourist.js, find the following code:

if(this._options.framework` == "bootstrap4"..... // if clause finishes on line 1835

$element.popover(popOpts); // this is currently line 1837

In between these 2 lines, on line 1836, make the following change to add the if clause:

if(this._options.framework` == "bootstrap4"..... 
}
// original clause finishes on line 1835

// ** ADD THIS if() ***
if(this._options.framework == "bootstrap4" && isOrphan == false)
{
    popOpts.selector = "#" + step.element[0].id;
}
// END ADDITION

// original code continues, this was line 1837
$element.popover(popOpts);

Please test that for me, it should work. I'll add the fix into the repo if so.

saviemedia commented 5 years ago

Yes it works now with the example mentionned above! Great thanks! I'm still wondering how to select an element inside a iFrame.

Example:

steps: [{ //element: "#MyElemInIFrame", // Doesn't work. //element: function() { return $("#MyElemInIFrame"); }, // Doesn't work. element: function() { return $("#MyIFrame").contents().find("#MyElemInIFrame"); }, // Doesn't work. title: "Help", content: "This is a test.", }]

Logs: [ Bootstrap Tour: 'ContenuOutil' ] Skip the orphan step 1. Orphan option is false and the element [object Object] does not exist or is hidden.

saviemedia commented 5 years ago

Here is some progress...

I was able to make this work:

steps: [{ element: function() { return $("#MyIFrame").contents().find("#MyElemInIFrame"); }, title: "Help", content: "This is a test.", }]

I had to create the tour inside that function:

$( window ).on( "load", function()... instead of $(document).ready(function()... because the iFrames and Images were not yet loaded.

The remaining problem is that the tour popup snap to the relative position of the element in the iframe. So, if the element is at the top in the iframe, the popup will appear at the top of the page. Do you know how take into consideration the absolute position of the element and not the relative position of the element in the iframe? I don't know if I'm clear...

Great thanks in advance! 👍

IGreatlyDislikeJavascript commented 5 years ago

I understand the problem. I think I need to add the iframe viewport offset into the position calculation for the popover and background.

I'll work on that add upload a fix to the repo when I have it done.

FezVrasta commented 5 years ago

@IGreatlyDislikeJavascript if you think the issue is with Popper.js please may you open an issue in its repository so I can take a look?

Thanks!

IGreatlyDislikeJavascript commented 5 years ago

@saviemedia I've updated the repo with a fix for the element: function() issue. I'm still working on the iframe issue.

@FezVrasta I've dug into this further. The issue isn't directly with popper.js, it's with the BS4 implementation of popper.js.

The popover options passed to Bootstrap are reconstructed internally into a new object before Bootstrap passes them to popper.js. Only a couple of popper.js options are actually transferred into the new object which is subsequently passed to popper.js. That means that many of the popper.js features are actually inaccessible when using BS4.

I thought the issue was that popper.js was ignoring the options I passed to it. The root cause is that those options were never passed to popper.js because of BS4.

However I'll consider opening an issue in your repo to enable an orphan/stick-to-viewport-center option in popper.js, which would be very useful.

Thank you.

IGreatlyDislikeJavascript commented 5 years ago

@saviemedia sorry for the delay on this, I've been frustratedly scratching my head over how to solve it.

The short version is, it's not easily solvable in BS4. Popper.js doesn't obey iframes, and BS4 uses popper.js without exposing it fully, so I currently think it's only possible to solve this if @FezVrasta fixes popover iframe element alignment, in popper.js.

I've opened a report in the popper repo (https://github.com/FezVrasta/popper.js/issues/791#issuecomment-522509816) so we'll need to wait for FezVrasta to take a look.

Once we have a solution from FezVrasta, I'll need to rework some of the positioning code in Tourist as well.

saviemedia commented 5 years ago

Great thanks for your proactivity. Please keep us posted if any changes. Thanks in advance to @FezVrasta as well. Very best!