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

Backdrop covers only the viewport and can be scrolled away #43

Open diesl opened 4 years ago

diesl commented 4 years ago

The title already says it, the backdrop covers only the viewport and can be scrolled away if the page has more content outside the viewport.

I my opinion, the backdrop should cover the whole page instead of only the viewport.

Example: docs.html

IGreatlyDislikeJavascript commented 4 years ago

Thanks for the report. Please let me double check this - I saw this bug previously, but it was caused by the underlying page having a broken/missing tag. You're right that the backdrop should cover the entire page.

Whilst I take a look, please can you revalidate your HTML on your live page and just make sure it's not a broken tag?

I know that docs.html has this issue, e.g.: line 556 is missing the closing > on the span (I just ran it through W3 HTML validator). That's because I had to manually grab and hack the Tour docs page.

diesl commented 4 years ago

Whilst I take a look, please can you revalidate your HTML on your live page and just make sure it's not a broken tag?

I tried a shortened docs.html with no HTML validation errors and got the same result.

I suppose it is simply a wrong CSS declaration. When I use position: fixed; on #tourbackdrop, it works fine.

IGreatlyDislikeJavascript commented 4 years ago

Great, thank you for the fix. I'll try and get that added before Christmas but, well, it's Christmas!

sdlins commented 4 years ago

@IGreatlyDislikeJavascript and @diesl,

The option position: fixed results in:

image

I tried to adjust the z-indez but with no success. The only workaround working for me is:

.tour-backdrop {
        height: 9999px;
}

That results in the follow while still covering the full page:

image

--- EDIT

Hmmm... not working everytime: image

IGreatlyDislikeJavascript commented 4 years ago

That second screenshot is odd, looks like the backdrop div has jumped above the popover zindex which shouldn't be possible if the only change you made was the height. I'll continue looking at this.

akfaew commented 4 years ago

.tour-backdrop { height: 9999px; } makes the whole page longer, and can be scrolled down to a now empty page.

diesl commented 4 years ago

Great, thank you for the fix. I'll try and get that added before Christmas but, well, it's Christmas!

Is there any progress on this? I wanted to replace bootstrap tour with bootstrap tourist, but currently there are too many critical bugs unfortunately ...

IGreatlyDislikeJavascript commented 4 years ago

Great, thank you for the fix. I'll try and get that added before Christmas but, well, it's Christmas!

Is there any progress on this? I wanted to replace bootstrap tour with bootstrap tourist, but currently there are too many critical bugs unfortunately ...

Hi @diesl , yes unfortunately my progress has been basically halted by the corona virus for various reasons. Offices mostly remain closed here (many businesses are continuing this into September/October), and I have no access to my development environment. It's also created some personal challenges, so I haven't been able to do anything about an alternative solution.

If there's an urgent fix that I can do with simple notepad / vim editing, I can do that. But investigating bugs and new functionality is more difficult.

No promises, I will try and do what I can, but appreciate your patience.

igor-stojcic commented 4 years ago

Regards to all. I managed to solve this problem in the following way ... In bootstrap-tourist.js around the line1817, in the if statement where the backdrop is appended, I added css rules to the body - position: relative;.

if ($(DOMID_BACKDROP).length === 0)
   {
        $(this._options.backdropContainer).append($backdrop);
        $('body').css('position','relative'); //add this for backdrop cover the whole page
   }
if ($(DOMID_HIGHLIGHT).length === 0)
    {
        $(this._options.backdropContainer).append($highlight);
    }

I hope that this solution will not disrupt the normal operation of the rest of the code. I ask the more competent members to give their comment, so that I and the others can learn :relaxed:

IGreatlyDislikeJavascript commented 4 years ago

@igor-stojcic I can't test this out currently, but although that looks like it could work, I'm not sure of the implications of changing CSS position for the entire DOM body . It feels like that could break things quite easily? I'm not a CSS expert though.

@diesl has posted a fix earlier in this thread, does that work for you? I want to figure out if diesl's fix is correct and sdlins has a specific local issue, or if diesl's fix doesn't work fully and sdlins is an example of that.

igor-stojcic commented 4 years ago

However, a better solution was given by @diesl with the position: fixed.

diesl commented 4 years ago

Hopefully I can also take a second look at this issue and the proposed solutions next week