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.

Sign up for GitHub

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

Jump to bottom

Keep track of events #1

Open
jankapunkt opened this issue Jan 3, 2019 · 6 comments
Open

Keep track of events #1

jankapunkt opened this issue Jan 3, 2019 · 6 comments

Comments

@jankapunkt
Copy link
Contributor

jankapunkt commented Jan 3, 2019 •
edited
Loading

When I teleport an element to the "outside" of my template, I am not able to listen to it's events using the Template's event-map anymore.

Most simple example:

<body>
{{>test}}
</body>

<template name="test">
  {{#Teleport destination='body'}}
    <button id="uniqueButton">click</button>
  {{/Teleport}}
</template>
Template.test.events({
  'click #uniqueButton' (event) {
    event.preventDefault()
    alert()
  }
})

Edit: the alert is expected to pop up here but it wont happen. Placing debugger won't trigger as well
Can you confirm this behavior? If so, any idea on how to fix this? I Would contribute if required.
@arggh
Copy link
Owner

arggh commented Jan 4, 2019

Currently Teleport is designed to work by including child templates (or atleast that is how I use it), like so:

<template name="app">
  {{#Teleport ''}}
    {{> randomComponent compProps }}
  {{/Teleport}}
</template>
Template.app.helpers({
   compProps() {
     return {
       onClick: () => alert('button clicked')
     };
   }

However, I think this should be considered a bug, as it's not obvious this won't work. Especially since you can use a similar pattern with normal Blaze block helpers and the event maps will be properly scoped.

Sidenote: I noticed that using ViewModel and it's event bindings your example pattern does work.

I can't yet put the time in to figure out the fix, but I'm very happy to accept a PR.

I think relevant bits of code in Blaze are:

https://github.com/meteor/blaze/blob/c6af95632b1a3f5a7077964e25c9be4ddaf7cc42/packages/blaze/builtins.js#L290

https://github.com/meteor/blaze/blob/611cff2570096b8abca47cddbb59dfff460de00f/packages/blaze/template.js#L124

@jankapunkt
Copy link
Contributor Author

jankapunkt commented Jan 4, 2019 •
edited
Loading

I would like to contribute but I want to be sure to not break things. Therefore I created a PR (see #2) that adds a minimal test suite. Could you please give it a shot?

@arggh
Copy link
Owner

arggh commented Jan 5, 2019

Merged #2 , next we need to add a failing test for this issue.

@jankapunkt
Copy link
Contributor Author

I created a new PR (see #3) which is currently only reflecting the test. If you think this will be sufficient as a test, I would start the implementation and add it as commit to this PR.

@arggh
Copy link
Owner

arggh commented Jan 7, 2019

We can add more tests when the first one passes, but one thing that could (possibly) be tested for, is to make sure we don't catch & propagate events from a possible child component inside the teleported block, as their events are handled separately.

<template name="app">
{{#teleport ''}}
   <button type="button">This we should take care of</button>
   {{> button onClick=thisIsAlreadyHandled }}
{{/teleport}}
</template>
Template.app.events({
  'click button'() { ... }
});

Regarding the implementation, I think we will have to setup event listeners by hand on the teleported block and manually bind them to the handlers specified in the Teleport's parent component.

I tried to come up with a solution last night and managed to make a flimsy proof of concept with the approach above, but I'm curious if you have already come up with a more elegant solution?

I also tried re-dispatching the cloned events inside the teleport's parent element, but that seemed to just create more problems.

@jankapunkt
Copy link
Contributor Author

I tried to come up with a solution last night and managed to make a flimsy proof of concept with the approach above, but I'm curious if you have already come up with a more elegant solution?

I was going to look into it by today but I first have to look deeper inside the whole tech under the hood. In my naive thinking I assumed, that I just had to change the root element for the querySelector and the template's internal jQuery instance from the template root to the new destination target. But I am not sure if the event maps in Blaze work this way so I first wanted to check this out until I get a full understanding of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jankapunkt @arggh