Skip to content

Conversation

@Art3miX
Copy link

@Art3miX Art3miX commented Oct 26, 2018

No description provided.

@Art3miX
Copy link
Author

Art3miX commented Oct 26, 2018

This feature allow you to set a target-less step on the middle of the page.

I wanted to show a message and let the user to choose if he wants to start the tour or not, but for that i had to manually create my welcome modal and do the buttons and handle the click, with this feature you simply add type: 'sticky' to the step params and it does the rest for you.

image

I'm sure more people will find it useful.

@mmorainville
Copy link
Member

Hi @Art3miX,

I reviewed your PR quickly and while it's working fine I think the code could be simplified.
Especially I don't think it's necessary to create a DOM element to attach the step to it.
We don't need to use Popper.js for a sticky step as we don't need to position it in relation to an another element.

I didn't tried it yet but ideally we could just remove all the code that adds/removes a temporary DOM element and just do something like:

// If the step is not sticky, we position it properly with Popper
if (!this.isSticky) {
new Popper(
          targetElement,
          this.targetElement,
          this.$refs['v-step-' + this.hash],
          this.params
        )
} else {
// Nothing to do here actually, we just display the step, and it should be done automatically in VTour.vue
// Of course we must keep the class logic `"v-bind:class="{ 'v-step-sticky': isSticky }"`
}

What do you think?

@mmorainville
Copy link
Member

mmorainville commented Nov 5, 2018

The only thing would be that if the dev has not put <v-tour> in App.vue but deeper in its DOM inside an element with position: relative it may cause some problems with the absolute positioning.
But in this case we could also just move the step directly in the else.

// If the step is not sticky, we position it properly with Popper
if (!this.isSticky) {
new Popper(
          targetElement,
          this.targetElement,
          this.$refs['v-step-' + this.hash],
          this.params
        )
} else {
// Move this.$refs['v-step-' + this.hash] (the step itself) in the body to be sure it's correctly positionned
}

@Art3miX
Copy link
Author

Art3miX commented Nov 15, 2018

Well, I didn't actually think about it, I wrote it that way because I didn't want to change the way how vue-tour works, and I wanted to leave the sticky step as a "real" step with a target because I don't know what you plan for its future, so i thought leaving it with a target would be better.

I will take a look into this and resolve the conflicts on the weekend.

@mattgreenfield
Copy link
Contributor

@Art3miX - do you think you will get a chance to finish this off? I'm thinking of submitting a similar PR myself

@mmorainville mmorainville reopened this Dec 30, 2019
@mmorainville mmorainville self-assigned this Dec 30, 2019
@HZooly HZooly mentioned this pull request Jan 31, 2020
@mmorainville mmorainville merged commit 9967a9b into pulsardev:develop Mar 27, 2021
@mmorainville
Copy link
Member

Hi, I've modified this PR a bit on another branch and merged it with this PR: #175.
What I've changed is that:

  • A step that has no target is considered sticky (i.e. no need for a isSticky option)
  • The sticky step is appended in the body (preventing incorrect behaviors with position: absolute containers)
  • The sticky step is fixed and centered in the viewport

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants