Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Step destroy #156

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

balazsnemeth
Copy link

When i created a step using createStep, it's going to automatically create the popup as well and insert it to the DOM. However I had no service to properly destroy/remove this step (returned by createStep), so I created a service.

@benmarch Let me know your thoughts if I need to structure my code on a different way or if something is missing. Thanks, and thanks for this library!

@benmarch
Copy link
Owner

Hey @balazsnemeth, this looks good to me, sorry I introduced the conflict. Can you add some test cases for these changes?

What do you think about making destroy() a method on the step itself instead of on the tour?

I think it would be good to call destroyStep() in the tourStepDirective when the scope is destroyed. What do you think?

Can you also revert the changes to the distribution file? I will generate that when I do the release.

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.

2 participants