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

removeEvent on Mouseup and before addEvent #306

Merged
merged 2 commits into from
Oct 25, 2017

Conversation

Robin-front
Copy link
Contributor

trace: #244

@coveralls
Copy link

Coverage Status

Coverage remained the same at 61.739% when pulling e8e046e on Robin-front:bugfix/removeEvents into caf5866 on react-component:master.

@monsieurnebo
Copy link

Any news about this PR?

@yvele
Copy link

yvele commented Sep 7, 2017

Can we help with this PR?

@paranoidjk
Copy link
Member

Sorry for the late reply. Thanks for you pr, I will take a look right now.

@@ -98,6 +98,7 @@ export default function createSlider(Component) {
this.dragOffset = position - handlePosition;
position = handlePosition;
}
this.removeDocumentEvents();
Copy link
Member

Choose a reason for hiding this comment

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

This will remove both TouchEvent and MouseEvent listener.

In mobile device, if there is no touchmove, the event will trigger follow touchstart --> touchend --> onmousedown --> onmouseup

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh..sorry. I am confused. Maybe I miss something.
clear event before addEvent, there is touchmove again. I don't know when that will happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean I remove both TouchEvent and MouseEvent listener, so there is no more touchmove, the event will trigger follow touchstart --> touchend --> onmousedown --> onmouseup;
But I just remove last event listener. In 103 lines, it add new event listener for this action. There is touchmove.

This bug happened when your mouse or touch move to another document(e.g. parent document with iframe). The event had bind in origin document, can not be remove correctly. So we have to check with extra func, to ensure we have a clean context.

Or I misunderstood. :D

@mpeula-pe
Copy link

Any progress in this PR?

@coveralls
Copy link

Coverage Status

Coverage remained the same at 61.739% when pulling d8abf46 on Robin-front:bugfix/removeEvents into caf5866 on react-component:master.

@paranoidjk paranoidjk merged commit 45975c4 into react-component:master Oct 25, 2017
@paranoidjk
Copy link
Member

close #244

@jitao13
Copy link

jitao13 commented Oct 25, 2017

The problem will be repeated on the PC.

<iframe src="http://react-component.github.io/slider/examples/slider.html" id="demo"></iframe>

Press the mouse to move the button, move the mouse over the iframe area, release the mouse, move back inside the iframe, and the button will still move with the mouse.

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.

7 participants