🐛 Fix initial scroll to top before scrolling to target #801
Conversation
| this.el.hidden = false; | ||
|
|
||
| this.tooltip.update(); |
There was a problem hiding this comment.
Are you sure we can remove this? Perhaps this was the wrong approach, but I think we needed to call update to ensure things got repositioned when scrolling or resizing the page etc.
There was a problem hiding this comment.
this seems to do extra renders, does the eventlistener for scroll call the initial show function?
There was a problem hiding this comment.
Unsure, but we need an update somewhere I think. Maybe I am wrong and popper now handles this internally?
There was a problem hiding this comment.
Does it update on scroll and resize without this?
There was a problem hiding this comment.
when it is out of viewport, there's a data attr added and we use that to set opacity to 0. What behavior where you expecting. Also, resize seems to have it's own internal events, as it does work as I'd expect.
There was a problem hiding this comment.
Just ensuring that the popper moves around and tries to stay in the viewport. I think on resize and scroll, the popper would stay in one spot before, if we weren't calling update.
There was a problem hiding this comment.
I checked this out locally, and it seems to be fine 👍
| @@ -29,7 +29,7 @@ describe('General Utils', function() { | |||
| } | |||
| }); | |||
|
|
|||
| const { popperOptions } = getPopperOptions(parseAttachTo(step), step); | |||
There was a problem hiding this comment.
scope the test to just one method
This fixes the issue with the document scrolling to top before each step is shown. Also, removes extra renders via the forced update in
_step()method.