Highlight overlay layer

[ibastevan]

I added the highlight layer but what i/we need to do now is set it so that it doesn't remove the backdrop or highlight on next step if next step backdrop is true. I'm not sure how to do this?
something like if next.step.backdrop ===false { $(".tour-backdrop").remove(); $(".tour-highlight").remove(); }
Then if we could find a way to append the existing highlight layer to update the style values on next step, it would transition nicely between steps.

[IGreatlyDislikeJavascript]

Thank you for working on this. I was just about to comment on the other thread :-)

Specifically to your q. Here's the current flow, for reference:

Showing a step:

1. showStep() is called
2. showStep() creates function showStepHelper() (line 1354) which is turned into a promise
3. showStepHelper() calls _showPopoverAndOverlay()
4. _showPopoverAndOverlay() calls _showOverlayElements() which is where the backdrop is created

(note, step 3 will call _scrollIntoView() -> _showPopoverAndOverlay() if page needs to be scrolled for element, but the result is the same)

Hiding a step:

1. hideStep() is called
2. hideStepHelper() is created as promise
3. _hideOverlayElement() is called, which removes the backdrop (and preventinteraction div)

So what we need to do is broadly:

1. Remove the $(".tour-backdrop").remove(); from _hideOverlayElement() so that the backdrop is never removed.
2. Break out the backdrop code from _showOverlayElements() into a separate _createBackdrop() function that creates the backdrop divs. We need to keep the preventInteraction stuff for now, can refactor that later
3. Add a new _removeBackdrop() function that removes the backdrop divs
4. Add functions to show and hide and position the backdrop divs
5. In tour.start(), call _createBackdrop()
6. In tour._showPopoverAndOverlay(), switch the call to this._showOverlayElements(step); on line 1710 to show and adjust the divs using the funcs from step 4

That should create the backdrop divs when the tour initializes, then simply update their positions on each tour step. Once that's working it's easier to refactor the code and make nice transitions.

Does that make sense so far?

[IGreatlyDislikeJavascript]

One more comment. I see you've updated the zindex in the css. We need to keep _fixBootstrapSelectPickerZindex() updated, specifically line 2218 zindex hack into the css, which solves a visual redraw problem with Bootstrap Selectpicker. That plugin custom draws the dropdown, and without the fix it appears incorrectly underneath the tour backdrop.

[ibastevan]

That is very clear instructions, thank you. I will have a go at that (probably tomorrow). I have been using the bootstrap selectpicker inside my application (along with many other bootstrap elements) so i will keep that in mind regarding z-index and function _fixBootstrapSelectPickerZindex().

Once this new backdrop is in place it should answer a lot of peoples prayers!

[ibastevan]

I am just working on this and it was nearly there, i just ran into a problem. I was hoping i could do this by creating a new function called createOverlayElements() and then inside the hide and showOverlayElements function, I would tell it what to do but hideOverlayElement is called on each step and it ends up hiding the backdrop div, where i want it to only hide on steps that have backdrop set to false. I tried to set an if statment. If step.backdrop then dont hide backdrop div but it doesnt seem to like that. Have you any thoughts?

[ibastevan]

With the _createBackdrop() function being placed in the tour.start() is that not telling it to create backdrop on each step?

[IGreatlyDislikeJavascript]

I'm not in front of my machine right now so excuse the short reply for the moment.

The solution is to completely remove any backdrop manipulation from the hide step flow, and remove the creation of the backdrop from the showOverlayElems.

Create the backdrop divs in a hidden state in tour.start, and remove them in the tour cleanup.

We want all backdrop manipulation (no creation or destruction) to happen in showStep / helper. ShowStep is the only place where we know previous backdrop state (through div visibility and previous tour step flow array), and current required state (because element function, delayOnElement etc will have all been called.

That's why I think we must only manipulate the backdrop on the showStep flow. In the new way of doing this, the hideOverlayElement should only manipulate a preventInteraction div (and we can refactor this later), and we must keep the backdrop divs and preventInteraction divs totally separate.

With the _createBackdrop() function being placed in the tour.start() is that not telling it to create backdrop on each step

No, we use _createBackdrop to create the backdrop in a hidden state on tour start, then manipulate/show/hide it in each step showStepHelper, which is where we know the result of element: function() or delayOnElement etc.

Does that make sense? Imagine the current overlay functions were renamed to showPreventInteraction and hidePreventInteraction, because they are not related to the backdrop any more and we'll refactor them later.

[ibastevan]

That does clear things up a bit. I need to do a bit more research into the showstep helper i think.
The preventInteraction code actually works better when used in the same way as the highlight div, as previously it wasnt postioning correctly (was too far left). I will try and get the backdrop and highlight working and leave the prevent div till later.

Thanks

[ibastevan]

I have pushed my changes if you want to take a look at what i have done.
It seems to be working pretty well. Although i think some of the css transitions may need revising.
Will need to test it with all other options too such as the _unfixBootstrapSelectPickerZindex.

[IGreatlyDislikeJavascript]

Fantastic, thank you! Let me take a look at this in detail tomorrow.

[IGreatlyDislikeJavascript]

This is really good, I love how it works and fixes both the visual issues/abrupt transitions, and also provides a way to solve the code challenges between steps.

Give me another day or so, I'm going to make some minor flow changes to your code and move a couple of things around.

Appreciate the confusion you ran into with the showStep flow and I can see why you put the preventInteraction code there - it took me a while to figure out why original Tour did things in a certain way, and some of it is legacy coffeescript transpilation oddness... So just need a couple of minor changes, I'll explain them in detail.

Thank you for the excellent contribution and ideas!

[ibastevan]

Thank you, I am glad you like it. Hopefully my code hasn't caused too many issues, it does take a lot to get your head around and knowing where things should go.
Looking forward to seeing final changes!
Thanks

[IGreatlyDislikeJavascript]

Okay, so I've made some changes and it looks like it's all working. Here's my updates:

CSS: https://pastebin.com/XxAv3iH4
JS: https://pastebin.com/z4nsXrCK

We need to take a look at the transitions as you said. I've also noticed something that I think is CSS related.

If you create a div with a black background and a span with white text, then use the span as a tour element, the appearance is "washed out". I'm not sure how to solve that as CSS etc isn't really my thing.

Can you take a look at that please? Once we've got that solved, we can work on some nice transitions - I'm thinking we add a backdropOptions {} to the tour settings and then give transition, formatting options etc...

I need to clean up and refactor some of the other code too, I'll do that once we've got the basics stable.

Let me know if you have any questions, thanks agaiN!

[IGreatlyDislikeJavascript]

Just to add, apologies for using pastebin but I'm not actually sure what will happen if I try to fork your fork of my fork, as I don't want to inadvertently break the Tourist in my repo whilst we implement these pretty significant changes!

[thenewbeat]

fork your fork of my fork

Git-inception

[IGreatlyDislikeJavascript]

@ibastevan I've added a new branch to the Tourist repo for this backdrop change, we can work on it there? (still learning how to use github :-(

https://github.com/IGreatlyDislikeJavascript/bootstrap-tourist/tree/unstable/new_backdrop

[ibastevan]

That’s great I will check it when I get back, I am away on holiday until 8th September.

[ibastevan]

I think the issue with elements with black backgrounds is that they would not require the highlight. So in that case perhaps a step option to set highlight to false or increasing the opacity from 0.9 to 0.2 or similar.

[ibastevan]

I wonder if there is a way to automatically get the background colour of the element and apply it to the highlight div?

[ibastevan]

Elements with coloured backgrounds will probably have issues with the type of backdrop and highlight layers. In which case the user may want to not have a highlight showing but will want to show the background color without the backdrop overlaying it.

We could add some jquery like -

$( "#tourHighlight" ).css( "background-color", color );

However this will not work very well if the element doesn't contain the background color and its inside one of it's parents.

We could even try something like this - http://jsfiddle.net/Y4uDL/2/

[IGreatlyDislikeJavascript]

I can see how your fiddle works, but I'm not sure we've defined the problem correctly. I'm really surprised if the way to solve this is to find the effective background color and set the layers based on that. I'm not exactly sure what's causing the washed out look.

[ibastevan]

The washed out look is due to the fact that the highlight layer has a white background set with an opacity and the elements have been brought forward using the z-index. This is what makes the element look like its clipped out from the overlay layer. This is what intro.js have done. Otherwise there is no other way.

Steps that contain dark background colours and light text would need to be manually adjusted on a per-step basis. We could just have a step option to change the opacity of the highlight layer. Steps with black backgrounds could set it to something like highlighOpacity: 0.2.

Although, If you have a step with the background colour of say: #0366d6. We could have another option for highlightColor: #0366d6. That would be ideal scenario and avoid any hacky solutions. Majority of users will probably have a whiteish background anyway. So these options would only really be used for the minority.

[ibastevan]

This seems to be working quite well now!
I think to make things easier for people we should look at finding a way to make the highlight position work with not having to add in this long line of code each time
domElement.width($(step.element).outerWidth()).height($(step.element).outerHeight()).offset($(step.element).offset());
It seems a bit daft having to put that in for each step you want to change the highlight transition effects!
maybe if we can make it a global variable or something like return fixHighlightPosition;

[IGreatlyDislikeJavascript]

Just need to note that the fadeIn() function would need to happen before the position is calculated otherwise it gets the wrong values.

Really? I need to check this. I thought we have to position it before fading in, or it will fade in the wrong position!

So in the Tour.prototype._updateBackdropElements function, i don't think we need this section as the tour-highlight-element doesn't apply to modals. We actually do need this section but it needs rewording as i think the if statement is no longer valid but the else statement is still valid.

Ok let's discuss this one. I still believe that the backdrop & highlight must be applied if the step.option.backdrop == true. Even if the step is a modal, we must apply the backdrop and highlight, because the tour creator asked us to. What do you think?

f i step from a reflex step to a modal step by clicking on a link open modal that opens up the modal, it does not find the step and attach the popover.

You're right, I don't know why that happens. It's very strange because as you say, the $.click approach works perfectly. I'll look into this one...

I think to make things easier for people we should look at finding a way to make the highlight position work with not having to add in this long line of code each time

I completely agree :-) . We need to fix the position before showing/hiding though, right? So I think it has to be a function that the tour creator can call if they want...

[IGreatlyDislikeJavascript]

Uploaded a new version to unstable/new-backdrop.

I think I've fixed the modal problem, please can you try it with the reflex option + "click on a link to open modal" approach?

I've also fixed the isLast problem. That was a major bug from original tour!

Haven't done the fixpositioning thing yet. Not sure of the best way to do it...

[ibastevan]

I have been out of work this last week but I am hoping to test this out properly next week,

[IGreatlyDislikeJavascript]

I've found a couple of bugs, there's a jquery error hiding the next button and the highlight isn't moving correctly between no backdrop and backdrop steps. Will work on those 2 tomorrow...

[ibastevan]

I can't seem to get it working for the reflex - modal step. Can you show me what your step options are for your working one?

[IGreatlyDislikeJavascript]

Sure, here's an extract from my test site.

Tour steps, without a forced modal open on onNext() (i.e.: modal launched by DOM button):

{
	element: "#btnOpenModal",
	reflex: true,
	reflexOnly: true,
	title: "Button",
	content: "Reflex step",
},
{
	element: "#exampleModal",
	backdrop: true,
	delayOnElement: {
						delayElement: "element"
	},
	title: "Modal",
	content: "this is a modal",
},

Tour:

var tour = new Tour({
						name: "tour",
						steps: tourSteps,
						container: "body",
						smartPlacement: true,
						keyboard: true,
						storage: window.localStorage,
						debug: true,
						backdrop: true,
						backdropContainer: 'body',
						backdropPadding: 0,
						duration: false,
						delay: false,
						basePath: "",
						showIfUnintendedOrphan: true,
						showProgressBar: true,
						showProgressText: true,
						framework: "bootstrap4"
					});

HTML:

<button id="btnOpenModal" type="button" class="btn btn-primary" data-toggle="modal" data-target="#exampleModal">
  Launch demo modal
</button>

And target exampleModal:

<div class="modal fade" id="exampleModal" tabindex="-1" role="dialog" aria-labelledby="exampleModalLabel" aria-hidden="true">
  <div class="modal-dialog" role="document">
    <div class="modal-content">
      <div class="modal-header">
        <h5 class="modal-title" id="exampleModalLabel">Modal title</h5>
        <button type="button" class="close" data-dismiss="modal" aria-label="Close">
          <span aria-hidden="true">&times;</span>
        </button>
      </div>
      <div class="modal-body">
        ...
      </div>
      <div class="modal-footer">
        <button type="button" class="btn btn-secondary" data-dismiss="modal">Close</button>
        <button type="button" class="btn btn-primary">Save changes</button>
      </div>
    </div>
  </div>
</div>

[ibastevan]

I think the issue is that .tour-highlight-element is being added to the modal which is causing the modal to show outside the screen. The reason being is bootstrap modal has a fixed position, whereas .tour-highlight-element has a relative position.

.tour-highlight-element should not be added to a modal element, if there is a requirement for it to be added in certain conditions, it may be best having it only added if backdrop is set to true, otherwise if backdrop is set to false then do not add .tour-highlight-element.

[ibastevan]

So in the Tour.prototype._updateBackdropElements function, i don't think we need this section as the tour-highlight-element doesn't apply to modals. We actually do need this section but it needs rewording as i think the if statement is no longer valid but the else statement is still valid.

Ok let's discuss this one. I still believe that the backdrop & highlight must be applied if the step.option.backdrop == true. Even if the step is a modal, we must apply the backdrop and highlight, because the tour creator asked us to. What do you think?

Yes it should be applied, even if it is a modal, however if you add the option "backdrop: false" then it shouldn't be applied, but we need to keep in mind .tour-highlight-element as that should not be applied either if "backdrop: false".

.tour-highlight-element needs to be part of the highlight function, i think this is getting forgotten about in places.

[ibastevan]

So,

// Is this a modal - we must set the zindex on the modal element, not the modal-content element
var $modalCheck = $(step.element).parents(".modal:first");
if($modalCheck.length)
{
	$modalCheck.addClass('tour-highlight-element');
}
else
{
	$(step.element).addClass('tour-highlight-element');
}

I think we need an && statement here, something like -
if(($modalCheck.length) && (step.backdrop == $(DOMID_BACKDROP).is(':visible')))
That should fix the issue.

[ibastevan]

I've found a couple of bugs, there's a jquery error hiding the next button and the highlight isn't moving correctly between no backdrop and backdrop steps. Will work on those 2 tomorrow...

What was the bug hiding the next button? That was working fine for me, only now the step popover isn't showing at all when i have reflexOnly: true. If i remove reflexOnly then it works, but then the next button shows.

In docs (line 196):

4. Only continue tour when reflex element is clicked
Use step option reflexOnly in conjunction with step option reflex to automagically hide the "next" button in the tour, and only continue when the user clicks the element

I'm not sure if this if statement is working (line 1903):

if(step.reflexOnly) 
{
    // Only hide the next button if this step is NOT an orphan
    step.template.find('[data-role="next"]').hide();
}

Or is this where the unintended orphan come into play? But then its not an orphan step. It has an element.

My step looks like this -

{   
    element: "#step",
    title: "Step",
    content: ``,
    placement: "auto",
    reflex: true,
    reflexOnly: true
}

UPDATE: If i add reflexOnly: true to any step that contains reflex: true in my tour it breaks the step.
ERROR MESSAGE: TypeError: step.template.find is not a function

[ibastevan]

Also, this code that has been added for isLast(). If the last step is an orphan, end the tour.

What if you don't want to end the tour? I have a scenario where the next page the user goes to requires the tour not to be ended as i have added some code to automatically begin the tour if the previous tour has not ended.

It's not much of a problem for me as i have just added a blank step after the orphan step so the tour doesn't end. Just wondering if this is the best way to do it, or if there should be a manual option added to the step such as onEnd: endTour or something.

[IGreatlyDislikeJavascript]

.tour-highlight-element needs to be part of the highlight function, i think this is getting forgotten about in places.

You're absolutely right, I don't know why I keep forgetting this even when you've reminded me so many times! For some reason I kept trying to hack it into the _updateBackdropElements(). I've now reworked this so that it is added in _showHighlightOverlay and _positionHighlightOverlay, and removed in hideHighlightOverlay. Hopefully this should fix it, because the show and position are only called if the backdrop is enabled.

What was the bug hiding the next button? That was working fine for me, only now the step popover isn't showing at all when i have reflexOnly: true. If i remove reflexOnly then it works, but then the next button shows.

ERROR MESSAGE: TypeError: step.template.find is not a function
That's the bug I was talking about. I added the unintended orphan handling, which meant I can't hide the Next button in tour._template() - because that function creates the tour step template/DOM code when the step is loaded. However an orphan isn't confirmed as unintended until (potentially) after the delayOnElement feature has timed out.

Now on line 1909 in the current version in the repo, is this code:
step.template.replace('data-role="next"', 'disabled');

This doesn't work though, because step.template is a string and not a jq object. I need to figure out a robust way to always hide the next button out of a string, remembering that we can only find it with data-role because the string is a customizable template.

So this bug isn't fixed yet.

It's not much of a problem for me as i have just added a blank step after the orphan step so the tour doesn't end. Just wondering if this is the best way to do it, or if there should be a manual option added to the step such as onEnd: endTour or something

So your code is checking localstorage and seeing if the previous tour ended? I have something similar, and I did it the same way you did - a dummy blank step after the "last" step which moves to the next page.

I see your point about adding an option to avoid the dummy step, but I'm not sure what the option would be or how it should be implemented. Logically if the tour gets to the last step, it's going to end :-)

[ibastevan]

You're absolutely right, I don't know why I keep forgetting this even when you've reminded me so many times! For some reason I kept trying to hack it into the _updateBackdropElements(). I've now reworked this so that it is added in _showHighlightOverlay and _positionHighlightOverlay, and removed in hideHighlightOverlay. Hopefully this should fix it, because the show and position are only called if the backdrop is enabled.

This is now working perfectly, good work!

This doesn't work though, because step.template is a string and not a jq object. I need to figure out a robust way to always hide the next button out of a string, remembering that we can only find it with data-role because the string is a customizable template.

I see, this looks like it could be a big job. Would the next, previous, resume and end buttons all need converting like the progressbar? We could then add in options such as showNext: false/true, showPrev: false/true, showEnd: false/true, showResume: false/true. Then in the step.reflexOnly we could have showNext: false?
It's a big job when we can just say

onShown: function (tour) {
                        $('[data-role="next"]').hide();
                    },

on the step.

So your code is checking localstorage and seeing if the previous tour ended? I have something similar, and I did it the same way you did - a dummy blank step after the "last" step which moves to the next page.

Yes, i had this in my new page that checks the storage. I thought that if a user decides to end the tour early, they wouldn't want to visit the new page and have the tour continue playing to them. I suppose i could just try and check the storage value and if the tour is on step X and ended then continue the tour, else do nothing.

My code looks like this -

if ((localStorage.XTour_current_step) && (!localStorage.XTour_end)) {
                $('#startTour').click(); // or could be tour.start(true);
            } 

I see your point about adding an option to avoid the dummy step, but I'm not sure what the option would be or how it should be implemented. Logically if the tour gets to the last step, it's going to end

The tour shouldn't just end, the user should be presented with the option to end the tour, or continue to the next tour/stage in the tour (this could be through "reflex" by clicking on a link on the page that will take them through to the next tour).

[ibastevan]

A also spotted another bug to do with the scrollbar and a plugin called DataTables. When my page loads, the highlight draws its position and shows behind the element (as it's supposed to).
But then my page loads a table of data causing the scrollbar to appear (which it's supposed to do).
But my issue is as the scrollbar appears it shifts the page to the left causing the highlights position to be off by the width of a scrollbar! It fixes if i resize the window, but on initial reoad of the page is where the issue appears.

The bug isn't really to do with DataTable plugin, more to do with the fact that the popover is loaded but not repositioned after scroll. It repositions after Window Resize, so may need something to tie in with that function for Scroll.

I don't know if you have any thoughts on this?

This hack fixes the tourHighlight position based on scroll event, but the popover is still off center to the element and it doesn't work doing this hack on the popover.

        $('html').scroll(function (){
            $('#tourHighlight').offset($('.tour-highlight-element').offset());
        });

[IGreatlyDislikeJavascript]

It's a big job when we can just say [...] on the step

Ah, you solved it for me, your code snippet gave me the idea. I've fixed this robustly now, by using jquery to find any DOM element with the role, then using the DOM code of the element to search & replace in the template. It's not ideal and there may be a better way, but it will make sure you never get stuck on a reflexOnly step that's an unintended orphan.

New version uploaded. We're almost there!

The tour shouldn't just end, the user should be presented with the option to end the tour, or continue to the next tour/stage in the tour (this could be through "reflex" by clicking on a link on the page that will take them through to the next tour).

I'm still not clear on this one. If the user gets to the last step of the tour, it will end. If there's "something" to continue to, then it's not the end of the tour?

Or are you saying that the code on line 1891 is the problem - clicking a reflex element on the last step of the tour calls tour.end() to be called, but the reflex element is designed to load another page which starts another tour. And that tour won't load, because tour.end() has set the local storage state of "tour has ended"?

I guess I'm still a little unclear exactly what behavior we should look at changing. I kind of get where you're thinking about the multi step/page tours, but I'm not clear on the impact for any other kind of tour - where getting to the last tour step doesn't end the tour.

A also spotted another bug to do with the scrollbar and a plugin called DataTables.

Yeah I use datatables as well. I bet it's because of the datatables responsive plugin/option, which does all sorts of stuff when the table data is loaded.

I'm having trouble replicating the issue you've found though, probably because my BS4 template is different to yours and my datatables are set up/loaded differently. Is there any way you could share some example code that causes it?

If resizing the window fixes it, I bet we can fix it with a trigger, but finding the right ones are going to be the issue.

Maybe we don't worry about fixing it in Tourist for the moment - maybe we just try and find what Datatables event we need to hook, and call $(window).trigger(resize) etc to try and make it happen totally separately. Then we can work out what to do in Tourist.

New version uploaded!

[ibastevan]

Or are you saying that the code on line 1891 is the problem - clicking a reflex element on the last step of the tour calls tour.end() to be called, but the reflex element is designed to load another page which starts another tour. And that tour won't load, because tour.end() has set the local storage state of "tour has ended"?

Thats correct! I can't start the next tour because it will only continue if the previous tour has not ended. Otherwise I wont know if the user actually wants to see the next part of the tour or not. Unless there is a better way of continuing tours across multiple pages and i am doing it in a silly way?

I'm having trouble replicating the issue you've found though, probably because my BS4 template is different to yours and my datatables are set up/loaded differently. Is there any way you could share some example code that causes it?

I'm using bootstrap3 but i will be upgrading to 4 in the next couple of weeks so perhaps this could be the issue... i will test it on 4 first and if still no luck i will send you some sample code.

I will test on Monday and let you know, thanks!

[ibastevan]

Pleased you managed to find a fix for reflexOnly... looking forward to testing it!

[IGreatlyDislikeJavascript]

Unless there is a better way of continuing tours across multiple pages and i am doing it in a silly way?

No, that's exactly how I do it as well. If you had one tour/tour steps and used the original Tour cross page functionality it works seamlessly. But I think you're don't it like me - finish a tour, move to one of N different pages, and "continue" the tour (really you're starting a new tour!).

I think we need to look at how to add an option to control that line 1891 behavior, but I'm not sure what at the min. It's easy to add an option like step.dontEndAfterThisStep, and that would solve it.

But what happens in an error case where tour creator wrongly sets that too true but doesn't then have another tour to continue (for whatever reason). You'd have a tour that's ended, but not ended in local storage, and therefore onPreviouslyEnded wouldn't trigger correctly.

....(scratches head)....

I will test on Monday and let you know, thanks!

Great, thanks for all your input and help, you've solved and added loads!

Let me know on bootstrap , I can't get it to happen on BS3 either. But BS3 uses built on popover whereas BS4 uses popper.js, so maybe there's another problem...

[IGreatlyDislikeJavascript]

I think to make things easier for people we should look at finding a way to make the highlight position work with not having to add in this long line of code each time

I've changed the way the step is passed to backdropOptions.animation.* highlight functions, to save that annoying positioning of the highlight element.

So now instead of having to do this in step.options.backdropOptions:

backdropOptions:
{
	highlightOpacity:			0.2,
	highlightColor:				"#000",
	animation:	{
					highlightShow:			function(domElement, step)
											{
												// annoying! domElement.width($(step.element).outerWidth()).height($(step.element).outerHeight()).offset($(step.element).offset());
												domElement.fadeIn();
											},

				}
},

You can call step.fnPositionHighlight() instead:

backdropOptions:
{
	animation:{
				highlightShow:	function(domElement, step)
								{
									// easy!
									step.fnPositionHighlight();
									domElement.fadeIn();
								},
			}
}

However there's another bug here. If you have the following scenario:

Step 1: anything (e.g.: intended orphan)
Step 2: element
Step 3: no backdrop

Then you go through the tour Step 1 -> Step 2 -> Step 3 -> Step 2 (i.e.: use Previous button on Step 3), the highlight is positioned wrongly visually.

What I don't understand is that although it's visually wrong, the width, height and offset are correct when checked in the DOM. I'm missing something, but not sure what.

step.fnPositionHighlight() will log the positioning to the console if tour options debug == true.

[ibastevan]

If you add the backdrop options as below to step before the step with no backdrop and the step after, it seems to work fine. It must be to do with the order of the fade and position. If fade goes first it seems to get position correctly.

backdropOptions:	{
		animation:	{
				 highlightShow:	function(domElement, step) {
					domElement.fadeIn();
					step.fnPositionHighlight();
                                 }
               }
}

[ibastevan]

You have fixed the scroll issue!

As for

But what happens in an error case where tour creator wrongly sets that too true but doesn't then have another tour to continue (for whatever reason). You'd have a tour that's ended, but not ended in local storage, and therefore onPreviouslyEnded wouldn't trigger correctly.
....(scratches head)....

I'm totally lost in what to suggest!
If someone were to add step.dontEndAfterThisStep and set to true when they havent setup a new tour to go to, they shouldn't really be adding that option in. This is only specific to people who genuinely don't want to end the tour.

[IGreatlyDislikeJavascript]

It must be to do with the order of the fade and position. If fade goes first it seems to get position correctly.

Ok cool, let's call that a jquery issue!

This is only specific to people who genuinely don't want to end the tour.

Yeah that's true. There might be a strange UX if for example the backend fails and doesn't load the next page, or the user refreshes the page. But the worst case there is that the last tour step is reshown, and when the user moves to the correct page a new tour will start/restart.

Ok, so I'll add a step.dontEndAfterThisStep (or some better name for it!) that controls this behavior.

Do we think that's everything fixed and stable? Are we ready to move this to the main branch now?

[ibastevan]

Yes all seems to be running very well now!

[IGreatlyDislikeJavascript]

Awesome, I'll move this to the main branch and update the docs (I need to sort the whole doc challenge out at some point).

Thank you for all your code, input and work on this, I literally had no idea how to implement the backdrop and fix the problems we ran into! Couldn't have done it without you.

[IGreatlyDislikeJavascript]

Updated to main branch and released as v0.3.0!

[ibastevan]

Well done for merging the branch and thank you! Please do let me know if there is anything you need help with or testing etc.

[ibastevan]

@IGreatlyDislikeJavascript thanks for all your hard work on this, unfortunately i am changing careers come January and so may not have any more time to help out with this repository. It looks like you have it covered now and plenty of other support here. Wishing you all the best for the future

[IGreatlyDislikeJavascript]

@ibastevan congratulations on your new career! Sorry to hear you won't be able to help out any more - you've made massive improvements and great contributions to Tourist, thanks for all your support and code, no way I could have done it without all your work. Good luck and all the best to you too!