Remove mouse events in Gesture Detector

RESOLVED FIXED

Status

Firefox OS
Gaia::System
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: janjongboom, Assigned: ramd, Mentored)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug])

Attachments

(1 attachment, 2 obsolete attachments)

46 bytes, text/x-github-pull-request
janjongboom
: review+
Details | Review | Splinter Review
In gesture_detector.js we currently have code to handle mouse events, but we already convert mouse->touch in the touch helper in gecko. This code can probably go.
(Reporter)

Updated

4 years ago
Whiteboard: [good first bug][mentor=janjongboom]
(Assignee)

Comment 1

4 years ago
Hi Jan, Can I get assigned to this bug?
And I have a doubt, do we need to remove js file and its link in all other files because it contains mouse events only.
Hi Vaishnav, I've assigned you to this bug.

The file 'shared/js/gesture_detector.js' contains some code regarding mouse-events (mousedown, mousemove, mouseup). That code can be removed. The gesture_detector.js file is the only file you'll need to touch.

Please ping me on irc.mozilla.org (#gaia or #b2g, @janjongboom) if you need assistance. Or post a new comment here and set 'Need more information from' to me.
Assignee: nobody → vaishnav.rd
Flags: needinfo?(vaishnav.rd)
(Assignee)

Comment 3

4 years ago
Thanks Jan, let me dive deeper into this file now.
Flags: needinfo?(vaishnav.rd)
(Assignee)

Comment 5

4 years ago
Created attachment 8350967 [details] [review]
pull request.txt

Pull request : https://github.com/mozilla-b2g/gaia/pull/14907
Attachment #8350967 - Flags: review?(janjongboom)
Sorry that I didn't take a look at this PR yet, it's due to the holidays. It's first thing on my list on January 2nd.
Comment on attachment 8350967 [details] [review]
pull request.txt

Thanks :Ram, almost there.

* Line 69: this.listeningForMouseEvents = true, should be removed, not commented out
* Line 65: this.options.mousePanThreshold = and 66, should be removed
* Line 165: GD.MOUSE_PAN_THRESHOLD = 15;, same
* Line 193: 'mousedown' should be removed from array
* function mouseCoordinates(e) { can be removed
* On top of the file, the description contains a lot of references to mouse. Can we remove that?

There is also a unit test in gallery that currently fails due to these changes, but I'll fix those before landing.

Can you make these changes, and re-push? Thanks!
Attachment #8350967 - Flags: review?(janjongboom)
(Assignee)

Comment 8

4 years ago
Created attachment 8355176 [details] [review]
PR

Thanks, I have fixed above things also, kindly have a look into new pull request.
Attachment #8350967 - Attachment is obsolete: true
Attachment #8355176 - Flags: review?(janjongboom)
Thanks Ram, I fixed the tests; so let's see if the build passes. https://travis-ci.org/mozilla-b2g/gaia/builds/16262193
Comment on attachment 8355176 [details] [review]
PR

r=janjongboom. :Ram, thanks for your contribution!

Landed in https://github.com/mozilla-b2g/gaia/commit/8b1c262f730d90e75cebcd4d6d56a5452111962a
Attachment #8355176 - Flags: review?(janjongboom) → review+
(Assignee)

Comment 12

4 years ago
Do I need to do anything else also ?
Flags: needinfo?(janjongboom)
I'll take a look at why the tests busted.
(Assignee)

Comment 14

4 years ago
Jan, Do you have any update on this?
Mentor: janjongboom@gmail.com
Whiteboard: [good first bug][mentor=janjongboom] → [good first bug]
Rebased the code and now running all the tests.
Flags: needinfo?(janjongboom)
Created attachment 8469154 [details] [review]
Rebased PR
Attachment #8355176 - Attachment is obsolete: true
Attachment #8469154 - Flags: review+
Try is green. Merged again https://github.com/mozilla-b2g/gaia/commit/44ee02c1691d5a42533727fdb5cc1fa628799a54

Sorry that it took so long for me to get back to this...
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.