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.
63 stars 36 forks source link

Bootstrap 4 popover container option (not a Tourist bug) #40

Closed RundleRomanowicz closed 5 years ago

RundleRomanowicz commented 5 years ago

First off, thank you for all the fixes bootstrap-tourist has done!

As I understand it, bootstrap-tourist "includes" the Bootstrap popover.js & tooltip.js libraries. This works great for my current project, as it requires a "Tour" as well as the use of popovers separately.

So my questions are as follows:

  1. Is my above assumption correct? In that including bootstrap-tourist should enable me to create a Tour asm well as popovers?
  2. The default "container" option doesn't seem to work when used in a popover context, in previous versions like Bootstrap3, the popover would render directly after the trigger button (button + div.popover) as a container. I can't seem to accomplish this when using bootstrap-tourist. As far as I know, this is default behaviour of Bootstrap4 as well, did something in this library break?
$('[data-toggle="popover"]').popover({
  title: 'HELP: File Size and Formats',
  content: '<strong>Image</strong> files should be a minimum of 2000x2000 pixels.',
  container: false,
});

or

$('[data-toggle="popover"]').popover({
  title: 'HELP: File Size and Formats',
  content: '<strong>Image</strong> files should be a minimum of 2000x2000 pixels.',
});

Should make the button element the "container"?

Any help would be much appreciated, or maybe I'm just misunderstanding it.

-Thanks!

ibastevan commented 5 years ago

Hi @RundleSG Have you included the bootstrap "bundle"? This contains the popover and tooltip code. From what i gather, we need to include these scripts for popover and tooltips to work. You can get them from the CDN here- https://www.bootstrapcdn.com/ To use bootstrap 4 version of the tour, you can add in the tour option framework: "bootstrap4"

I am sorry i cannot be any more of help, but I am just in the process of transitioning my tour over to BS4, I am sure someone with some better understanding will provide you with some help soon.

Thanks

RundleRomanowicz commented 5 years ago

Hi @ibastevan, Thanks for the help! I appreciate it.

Yes I have included the bundle, as bootstrap-tourist is not standalone (I should have mentioned that in my post).

My popups and tours function perfectly otherwise. I've included some screenshots the markup from the official BS3 & BS4 documentation, hopefully you can see the difference. Does that make more sense?

Screen Shot 2019-11-01 at 11 00 10 AM Screen Shot 2019-11-01 at 10 58 21 AM

Thanks!

ibastevan commented 5 years ago

So the way bootstrap tourist works, is the popover html is generated on the fly based on the steps you create in the step array. Here is an example of what my tour code looks like:

I start my tour by clicking on a button with the ID startTourBtn

            $('#startTourBtn').click(function () {
                tour.start(true);
            });

My steps go inside the TourSteps array

var TourSteps = [
                {   //step 00
                    element: "#Step1",
                    title: "Welcome to my Tour",
                    content: `This is a tour`,
                    placement: "auto",
                    reflex: true,
                    preventInteraction: false
                }];

This is my code for the tour setup:

var tour = new Tour({
                name: "tour",
                steps: TourSteps,
                backdrop: true, 
                framework: "bootstrap4",   
            });

In regards tp the popover html, that is autogenerated in tourist from the objTemplates.

bootstrap4  : '<div class="popover" role="tooltip"> <div class="arrow"></div> <h3 class="popover-header"></h3> <div class="popover-body"></div> <div class="popover-navigation"> <div class="btn-group"> <button class="btn btn-sm btn-outline-secondary" data-role="prev">&laquo; ' + this._options.localization.buttonTexts.prevButton + '</button> <button class="btn btn-sm btn-outline-secondary" data-role="next">' + this._options.localization.buttonTexts.nextButton + ' &raquo;</button> <button class="btn btn-sm btn-outline-secondary" data-role="pause-resume" data-pause-text="' + this._options.localization.buttonTexts.pauseButton + '" data-resume-text="' + this._options.localization.buttonTexts.resumeButton + '">' + this._options.localization.buttonTexts.pauseButton + '</button> </div> <button class="btn btn-sm btn-outline-secondary" data-role="end">' + this._options.localization.buttonTexts.endTourButton + '</button> </div> </div>',

You can change the popover html by adding the template option to the tour setup:

template: `put your popover html here`

So it would look like:

var tour = new Tour({
                name: "tour",
                steps: TourSteps,
                backdrop: true, 
                framework: "bootstrap4",  
                template: `put your popover html here`
            });

This is my bootstrap3 example template html:

template: `<div class='popover tour border-slate'><div class='arrow'></div><h3 class='popover-title bg-slate'></h3><button class='btn btn-end' data-role='end'><i class="icon-cross2"></i></button><div class='popover-content'></div>
                            <div class='popover-navigation text-right'><button class='btn btn-flat text-muted' data-role='prev'>Previous</button>
                            <button class='btn btn-primary' data-role='next'>Next</button></div></div>`

Does any of this help at all?

IGreatlyDislikeJavascript commented 5 years ago

@ibastevan awesome reply as always, thank you :-)

@RundleSG the only thing I can add is that Tourist uses the native Bootstrap (3 and 4) popover functionality. If you use BS3, it's popover. If you use BS4, it's popper.js.

So the positioning of the code in your DOM is determined by Bootstrap, not by Tourist. Tourist simply calls element.popover() to show each step against a DOM element (https://getbootstrap.com/docs/4.0/components/popovers/#methods)

You can see this code on line 2253 - the same code applies to BS3 and BS4, however the options are constructed slightly differently (line 2194).

Does this help?

IGreatlyDislikeJavascript commented 5 years ago

Probably should add that the global container and step.container options simply pass their values directly through to the Bootstrap popover. Line 2204:

container: step.container

RundleRomanowicz commented 5 years ago

@ibastevan @IGreatlyDislikeJavascript Thanks for bearing with me... I'm trying to accomplish 2 things here, which may be my problem. 1) Create a tour (using BS4 which I defined in the tour with the framework key. The tour actually functions fine so far, and worked right after dropping bootstrap-tourist in. :D

2) Additionally, I'm creating multiple "popovers" (see screenshot). Screen Shot 2019-11-01 at 12 54 34 PM

These also function out of the box, the only issue i'm having with is the container: 'body'. option. (see my first reply to @ibastevan with the two included screenshots) To explain a bit more, I'm trying to position the generated popovers directly after the relevant button in the HTML render order. (also tried to explain in screenshots in my above reply).

Does that clear anything up? If not, don't worry I'll find some other solution.

Thanks again!

EDIT: Code used to generate content for popups (completely separate and independent from the steps & options for the tour)

jQuery('[data-toggle="popover"].inline-popover.multi--img-main.help').popover({
  title: 'HELP: File Size and Formats',
  content: '<strong>Image</strong> files should be a minimum of 2000x2000 pixels, this is to ensure the resolution is high enough that they do not appear grainy or blurry on all devices. The maximum file size is 25 MB, but it is recommended to adjust the image size before uploading. Allowed file types include: png, gif, jpg or jpeg.',
  html: true,
  container: false,
});
ibastevan commented 5 years ago

I think I’m starting to understand what you’re trying to accomplish. Basically your popovers (separate to tourist popovers) are not popping up next to the buttons? Or is the tourist popovers not showing in the right place?

ibastevan commented 5 years ago

For tourist you can set the position using placement: "auto" I’m not all clued up yet how it works with BS4.

IGreatlyDislikeJavascript commented 5 years ago

So it's the DOM code position, not the UX visual? Can I ask why you need the popover to appear next to the element in the DOM code? If the popover is visually appearing in the right place, is there another wider issue?

The container option does get passed direct to popper.js (BS4), so either it's not working in the Bootstrap/popper code (I'm not surprised if so, I've already found and reported bugs with popper.js and BS4), or I've missed something in Tourist code.

Actually can I ask one silly question - is your issue related to the Tourist tour popovers, or the other BS4 popovers that you're creating with the code you posted?

RundleRomanowicz commented 5 years ago

@ibastevan Thanks, they're rendering okay. It's more of the placement of markup in the DOM of the popovers. The way they currently render, makes it really hard to do any conditional CSS.

<body>
 <div id="inner1">
     <div id="inner2">
        <a class="inline-popover event--event-type help" data-toggle="popover" tabindex="0" data-trigger="focus" data-original-title="" title=""><span class="icon"></span>Event types explained: </a>     
     </div>
</div>

<div class="popover popover-help fade bs-popover-bottom show" role="tooltip" id="popover779729" x-placement="bottom">
  <!-- inner popup template and contents would be here -->
</div>

</body>

I would like the popup to render in the #inner2 div, after the a.inline-popover

@IGreatlyDislikeJavascript I'm trying to determine where the issue stems from, it worked fine with bootstrap-tour, I dropped in bootstrap-tourist and the required BS4 bootstrap.bundle.min.js and bootstrap.min.js, then the positioning of the popovers in the DOM changed.

I'm still trying to get up to speed with the BS4 changes as well.

IGreatlyDislikeJavascript commented 5 years ago

Right I got it now! So I'll need to trace through the code and try to figure out why it's not being added to the DOM in the right place. It could well be another issue with popper.js and bootstrap.

Just so I'm absolutely clear: does the "wrong code position in DOM" happen if you use BS4 popover functionality as well? Or does it only happen to Tourist popovers, and regular native BS4 popovers are placed in the code correctly?

RundleRomanowicz commented 5 years ago

@IGreatlyDislikeJavascript Okay awesome!

Just so I'm absolutely clear: does the "wrong code position in DOM" happen if you use BS4 popover functionality as well? Or does it only happen to Tourist popovers, and regular native BS4 popovers are placed in the code correctly?

Crap, maybe I explained it wrong again. The code snipped I provided in my comment above and then the DOM example in my other reply is for generating the BS4 popovers. Tours are separate and I did not include that code as it's not giving me issues right now.

Does that make anymore sense or am I confusing you more?

This issue is only happening with BS4 popovers, I have 0 issues with the tour element right now. But since bootstrap-tourist / bootstrap.bundle.min.js / bootstrap.min.js drive the Tour & BS4 popovers, it might be all related? I haven't had a chance to test every single Tour option thoroughly yet, but I don't believe there are any issues there.

IGreatlyDislikeJavascript commented 5 years ago

This issue is only happening with BS4 popovers, I have 0 issues with the tour element right now.

Okay, got it! I could be brutal and just close this as it's not a Tourist issue, but I'm not that cruel ;-)

I think I may have an idea. Now I've looked at your jquery code again, try changing the container option to a DOM element. You shouldn't set this to bool, it's expecting an element id. See here: https://getbootstrap.com/docs/4.0/components/popovers/#example-using-the-container-option

Let me know if that works.

RundleRomanowicz commented 5 years ago

If it's not a tourist issue (which you confirm) I can create an issue over on the BS repo. I just wasn't sure if anything was changed, I was under the impression that the code was forked and tweaked for tourist, which I now know not to be the case.

I think I may have an idea. Now I've looked at your jquery code again, try changing the container option to a DOM element. You shouldn't set this to bool, it's expecting an element id. See here:

I did try that, false and body seem to give the same result. In BS3 if I set it to false i believe it automatically appended the popover to the button (which is what I want).

If I have 20 popovers on the page, I don't know which DOM element I should be using, as I want each popover appended to their relevant buttons.

Thanks for all your help, I don't blame you if you just close this as it's not a tour problem. Regardless, I really appreciate the input from both of you @ibastevan @IGreatlyDislikeJavascript .

ibastevan commented 5 years ago

That’s an interesting way of setting it, I tend to just go by the popover ID for any conditional css but I can see how that would make things easier being in the buttons wrapper. Very good to know as I didn’t even know this was possible!

Very interested to see if we can get this working!

RundleRomanowicz commented 5 years ago

@ibastevan Now you're making me doubt myself lol. I will verify tonight that BS3 popovers act that way.

IGreatlyDislikeJavascript commented 5 years ago

Okay I have this working perfectly using default BS4 functionality, so it's not a BS4 bug - I think its your local scenario or code. I can't create a fiddle because we need to look at the live code, but in short here's all I did:

First I created a BS4 project using latest jquery and BS4 from CDN. I also started with sbadmin template just to have something real-world as a base.

Next I added a dom element inside a div to the page:

<div id="divPopover">
    <p id="popoverTargetA" data-toggle="popover">
        A popover here
    </p>
</div>

Then I enabled popovers and set the content, just like you've done:

$('[data-toggle="popover"]').popover({
            html: true,
            title: "A popover",
            content: "Popover body",
            trigger : 'hover',
            placement: "auto",
            container: "#divPopover"
        });

And the result is a popover, with the DOM code looking like this, exactly as expected - popover HTML added to the div specified as the container.

Untitled

If I change the js code to this so the container is programmatically found:

$('[data-toggle="popover"]').popover({
            html: true,
            title: "A popover",
            content: "Popover body",
            trigger : 'hover',
            placement: "auto",
            container: $(document).find("#divPopover")
        });

It also works fine, so my assumption is that any similar jquery object with one object in the collection will work ($.parent etc).

If I set container: false, the popover code is added to the DOM at the end of body (which is also what happens if I omit container, or set an invalid value).

So at this point, unless I've misunderstood the problem again (very likely :-) ), the problem is in either your code, or the version of Bootstrap you're using. I have noticed issues with some previous CDN versions of Bootstrap that I never fully understood...

Happy to keep this open for a little bit to see if we can help, but I don't think we can add much more value and you might get more mileage out of a stackoverflow post or similar.

ibastevan commented 5 years ago

It might be worthwhile seeing all the scripts and versions on your page. I had issues with jquery 2.1.4 so there could very well be a conflict somewhere. If you can do a fiddle that would be even better

RundleRomanowicz commented 5 years ago

@IGreatlyDislikeJavascript @ibastevan Thank you both very much for your thoughts. Sure maybe leave it open for a bit, I'll post any of my findings from further experimenting.

RundleRomanowicz commented 5 years ago

I've created a few codepens to tests this out.

1) Vanilla BS3 popovers

2) Vanilla BS4 popovers

3) popovers & tour with popper + BS4 +bootstrap-tourist

Anyways this is just what I put together to test and give proper examples.

Thanks again guys.

IGreatlyDislikeJavascript commented 5 years ago

Okay, now I really do understand :-) . In short, BS3 popovers have the popover DOM code added as an immediate sibling to the element they are describing. In BS4, even with the container option, popper.js appends the popover code to the element or container parent.

Here's the bad news: what you want isn't possible "natively". Bootstrap 3 uses popover, Bootstrap 4 uses popper.js. They behave fundamentally differently, and there are some issues with BS4 / popper.js.

I don't believe there's a way to make popper.js add the popover to the DOM as next sibling, using options. What's more, even if there was a popper.js option to do it, BS4 wraps popper.js in such a way that you can't access that option. BS4 actually recreates your popover options by copying selected keys to a new object, then passing that object to popper.js. So not all of popper.js features (in fact very few of them) are accessible through native BS4 popovers.

I think your only solution will be to programmatically add a div (before/after/sibling/etc) to every element that has a popover, then specify that div as the container. Then you'll be able to maybe do your conditional code, depending on what you want. Something like this?

// create sibling divs next to your element or some other jq collection
$("<div class='adjacentForPopper'></div>").insertAfter("#myElement");

// set up popovers on your element/jq collection, with containers set to adjacent div
$("#myElement").popover({
        title: "A popover",
        content: "Popover body",
        trigger : 'hover',
        container: $(this).next(".adjacentForPopper");
    });

$("#myElement").popover("show");

I can't see any other solution to this, other than editing popper.js code and adding a next sibling option yourself. Good luck!

RundleRomanowicz commented 5 years ago

@IGreatlyDislikeJavascript Wow you went above and beyond, thank you for that. Okay that makes sense, I wasn't aware of all that regarding BS4 (I'm really still getting up to speed on it, Drupal has been a bit slow to adopt BS4). I'm going to give your solution a try tomorrow morning, maybe it could help someone if they ever have this specific issue (doubtful).

Really appreciate all your help!

ibastevan commented 5 years ago

@IGreatlyDislikeJavascript that is a brilliant work around, I am sure this be useful for others. @RundleSG thank you for creating those examples on codepen, i am sure they too will be of use for lots of people here!

I remember you mentioning earlier that you need the popover inside the #inner div so you can write some conditional css for it. I was just wondering if it would not be easier to just create a css class with the styling inside that and then just apply the class to the popovers in question?

Adding styling to #ID's is not ideal (although we all do it at some point!), because you cannot reuse the code in other places, and as your site grows it becomes more difficult to manage.

Anyway it was just a thought and it may not apply to what you are trying to accomplish and @IGreatlyDislikeJavascript workaround should work well. :)

IGreatlyDislikeJavascript commented 5 years ago

Closing this one as I think we've kind of solved it, and it's also not a Tourist issue!