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

Detach instance #72

Merged
merged 11 commits into from
Aug 3, 2018
Merged

Detach instance #72

merged 11 commits into from
Aug 3, 2018

Conversation

mrsweaters
Copy link
Collaborator

@mrsweaters mrsweaters commented Aug 18, 2017

Work in Progress.

This is an attempt at detaching Tribute from a bound instance. However, the event clean up is not working quite right. Some help would be appreciated!

Should address:
#69
#37

Jordan Humphreys and others added 5 commits August 18, 2017 09:35
* Allow for manually triggering menu.

* More work on trigger-menu.

* more work on trigger menu.

* Get manual activation working.
@omarstreak
Copy link

You are unbinding events from the passed in element, but you're not unbinding the events from the document and window.

@omarstreak
Copy link

Also, when you say the event cleanup isn't working quite right, can you give more details about what is going on?

@mrsweaters
Copy link
Collaborator Author

@omarstreak I pushed an update that removes the global window object bindings. When I said it wasn't working quite right, I meant that there is currently an issue with rebinding to an element after tribute has been detached. I'm currently looking for what might be the cause of that. For the most part, it appears to be working.

@biallo
Copy link

biallo commented Apr 12, 2018

Hello, when can this PR be merged?

@mrsweaters
Copy link
Collaborator Author

@biallo I should have some time to merge this in on Monday. Thanks for bringing it to my attention!

src/Tribute.js Outdated
setTimeout(() => {
el.removeAttribute('data-tribute')
this.isActive = false
this.menu.remove()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my usage scenario, after the user opens the input box, it does not necessarily use '@'. At this time, if I use 'detach', an error occurs because the menu is 'undefined' at this time.

if (this.menu) {
  this.menu.remove()
}

}

}

unbind(menu) {
menu.removeEventListener('keydown',
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my usage scenario, after the user opens the input box, it does not necessarily use '@'. At this time, if I use 'detach', an error occurs because the menu is 'undefined' at this time.

if (menu) {
  menu.removeEventListener('keydown', this.menuKeydownEvent, false);
}

@origo27
Copy link

origo27 commented Aug 1, 2018

any updates?

@mrsweaters mrsweaters merged commit aed62f0 into master Aug 3, 2018
@mrsweaters mrsweaters deleted the detach-instance branch August 3, 2018 22:28
this.menuEvents.unbind(el.tributeMenu);

setTimeout(function () {
el.removeAttribute('data-tribute');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the hard work on this feature.

I'm getting a warning from the attach function when I use the detach and attach function back to back:

tribute.detach(element);
tribute = new Tribute({
    collection: newValue
});
tribute.attach(element);

Tribute was already bound to DIV

I suspect it's the setTimeout function doing this, but was there a reason for it?

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

Successfully merging this pull request may close these issues.

5 participants