tsers-js / core

Transform-Signal-Executor framework for Reactive Streams
MIT License
145 stars 4 forks source link

Event listener binding bug in React DOM interpreter #22

Closed ghost closed 8 years ago

ghost commented 8 years ago

I was trying to port this code to TSERS. As you can see, it's a very simple drag&drop example wrtitten with rxjs. My code looks like this:

import {Observable as O} from "rx"

export default function main(signals) {
  const {DOM: {h, prepare, events}, model$, mux} = signals

  const vdom$ = prepare(model$.map((state) =>
    h('div', {
      id: 'mydiv',
      style: {
        backgroundColor: 'black',
        width: '70px',
        height: '70px',
        display: 'inline-block',
        position: 'absolute',
        cursor: 'move',
        top: state.top + 'px',
        left: state.left + 'px'
      }
    })
  ))

  const mousedown$ = events(vdom$,'#mydiv', 'mousedown')
  const mouseup$ = events(vdom$, '#mydiv', 'mouseup')
  const mousemove$ = events(vdom$, null, 'mousemove')

  const mousedrag$ = mousedown$
    .flatMap(function(md) {
      const
        startX = md.nativeEvent.offsetX,
        startY = md.nativeEvent.offsetY;

      return mousemove$.map(mm => {
        mm.preventDefault();

        return {
          left: mm.clientX - startX,
          top: mm.clientY - startY
        };
      }).takeUntil(mouseup$);
    })

  const mod$ = mousedrag$.map(pos => state => pos)

  return mux({
    DOM: vdom$,
    model$: model$.mod(mod$)
  })
}

It works OK except the first time you "mouseup" (free mouse button). These are the steps to reproduce this issue: 1) mousedown over item (mydiv) and mouseup (=mouseclick) 2) move your mouse. mydiv moves!!! Item sould not move if you are not pressing your mouse button. If you try it again (without reset app, F5), it works OK. It's like the program listen all mouseup events but the first one. I used ReactDOM. I've tried Snabbdom interpreter but it doesn't work (no styles?).

Thank you in advance.

milankinen commented 8 years ago

Hi @pmros and thanks for reporting this. This indeed seems like a bug in event listener binding in React DOM interpreter implementation.

I ported your example @tsers/snabbdom and it's working fine. It was nice notice that styles weren't working with @tsers/snabbdom. I had a small bug there and fixed it in version 0.4.1.

The code looks like this:

import {Observable as O} from "rx"

export default function main(signals) {
  const {DOM: {h, prepare, events}, model$, mux} = signals

  const fullScreenStyle = {
    position: "absolute",
    top: 0,
    left: 0,
    width: "100%",
    height: "100%"
  }

  const vdom$ = prepare(model$.map((state) =>
    h("div#wrapper", {style: fullScreenStyle}, [
      h('div#mydiv', {
        style: {
          background: 'black',
          width: '70px',
          height: '70px',
          display: 'inline-block',
          position: 'absolute',
          cursor: 'move',
          top: state.top + 'px',
          left: state.left + 'px'
        }
      })
    ])))

  const mousedown$ = events(vdom$, '#mydiv', 'mousedown')
  const mouseup$ = events(vdom$, '#mydiv', 'mouseup')
  const mousemove$ = events(vdom$, '#wrapper', 'mousemove')

  const mousedrag$ = mousedown$
    .flatMap(() => mousemove$
      .map(mm => {
        mm.preventDefault();
        return {
          left: mm.clientX,
          top: mm.clientY
        };
      })
      .takeUntil(mouseup$))

  const mod$ = mousedrag$.map(pos => state => pos)

  return mux({
    DOM: vdom$,
    model$: model$.mod(mod$)
  })
}
ghost commented 8 years ago

@milankinen Thank you! Now it works (with Snabbdom). I'll keep playing with TSERS (this time with SVG, I know there is a open issue about that).

Note: Drag&drop experience is better if you code

 const mousemove$ = events(vdom$, null, 'mousemove')

instead of

 const mousemove$ = events(vdom$, '#wrapper', 'mousemove')
ghost commented 8 years ago

I've shared mousedown$, mouseup$ and mousemove$, and now it works OK!

const mousedown$ = events(vdom$,'#mydiv', 'mousedown').share()
const mouseup$ = events(vdom$, '#mydiv', 'mouseup').share()
const mousemove$ = events(vdom$, null, 'mousemove').share()
milankinen commented 8 years ago

You mean it works with @tsers/react too?

ghost commented 8 years ago

Yes! That's exactly what I mean! :smile: