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

Non-bubbling event handler stops working after redraw #853

Closed
lolgesten opened this issue Oct 18, 2018 · 6 comments
Closed

Non-bubbling event handler stops working after redraw #853

lolgesten opened this issue Oct 18, 2018 · 6 comments

Comments

@lolgesten
Copy link

It seems like non-bubbling event handlers are not added back to the DOM when underlying DOM nodes are removed/re-added.

Code to reproduce the issue:

const xs = require('xstream').default;
const {button, form, makeDOMDriver} = require('@cycle/dom');
const {run} = require('@cycle/run');

function main(sources) {

  const formSubmit$ = sources.DOM.select('form')
    .events('submit', { preventDefault: true })
    .mapTo({});

  const notformClick$ = sources.DOM.select('.notform')
    .events('click')
    .mapTo({});

  const state$ = xs.merge(formSubmit$, notformClick$).fold(p => !p, true);

  const vdom$ = state$.map((showForm) =>
    showForm
      ? form([button("Form button")])
      : button('.notform', {props: { type: 'button' }}, "Not form button", )
  );

  return {
    DOM: vdom$
  };
}

run(main, {
  DOM: makeDOMDriver('#app')
});

Expected behavior:
Click the button should just switch the DOM between <form><button/></form> and <button>.

Actual behavior:
On the third click, the submit event is not intercepted, instead it goes to <form> and the entire page reloads.

Inspecting the code, I see fromEvent.js row 22 element.addEventListener is done on the <form> element because submit is classified as non-bubbling. But the event listener is not added back after the form element is gone and then redrawn.

Versions of packages used:
@cycle/dom: 22.0.0
@cycle/run: 5.1.0
xstream: 11.7.0

@jvanbruegge
Copy link
Member

This might be related to #839
I have to investigate this, thanks for the repro

@IssueHuntBot
Copy link

@issuehuntfest has funded $60.00 to this issue. See it on IssueHunt

@jvanbruegge jvanbruegge added this to Short term TODO in jvanbruegge's pipeline Dec 4, 2018
@shesek
Copy link
Contributor

shesek commented Mar 22, 2020

I've been running into this issue as well and its been driving me crazy. Are there any workarounds?

@jvanbruegge
Copy link
Member

It is possible that #906 fixes this as well, I will cut a release today or tomorrow so you can try it out

@shesek
Copy link
Contributor

shesek commented Mar 22, 2020

@jvanbruegge I tested against master, this indeed solves it! Thank you 🙏

@jvanbruegge
Copy link
Member

good to know, thank you!

shesek added a commit to Blockstream/esplora that referenced this issue Mar 24, 2020
This was needed due to a cyclejs bug that is now fixed
in @cycle/dom v22.6.0 by cyclejs/cyclejs#906

This also fixes some bugs in other forms that didn't implement the hack,
like the push transaction form which didn't work after hitting 'back'.

Also see cyclejs/cyclejs#853
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
jvanbruegge's pipeline
Short term TODO
Development

No branches or pull requests

4 participants