635629 - Support mouse gestures in calendar view

Status: RESOLVED FIXED

(Calendar :: Calendar Frontend, defect)

Description

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v1

On Mac (and as I've heard also Windows 7) its possible to do certain mouse gestures: Swipe, Rotate, Zoom. This patch adds support for those gestures in the calendar views.

Comment 1

[Philipp Kewisch [:Fallen][☁️📆]]

Comment on attachment 513882 [details] [diff] [review]
Fix - v1

I just found out someone has a Mac ;-) This patch should still apply, if not let me know, I have one locally that does.

Comment 2

[Mohit Kanwal [:redDragon]]

I tested it on my Mac ;) ! Th gestures work fine on 10.7 Lion. Swipe is the most responsive of the lot. Rotate seems a bit sluggish may be it is coz of the time and sudden nature of the animation when it swaps the time to the day in the view. May require a bit of tweaking. Usually I can rotate with one hand, and dont have to necessarily rotate it by 180 degrees.

Zoom is also cool, rendering seems to be a bit slow though. I don't know if we can improve that.

Will there be a changelog dialog for Mac Users when this patch is pushed to the next release? Something like Skype [1]. 

[1] http://www.abnormalmarketing.com/wp-content/uploads/2010/10/Facebook-and-Skype.jpg

Comment 3

[Mohit Kanwal [:redDragon]]

Comment on attachment 513882 [details] [diff] [review]
Fix - v1

Review of attachment 513882 [details] [diff] [review]:
-----------------------------------------------------------------

Just have a few questions about the use of constant values in the patch. I will change to '+' once i get the answers :)

::: calendar/base/content/calendar-base-view.xml
@@ +820,5 @@
>          event.preventDefault();
>        ]]></handler>
> +      <handler event="MozRotateGesture"><![CDATA[
> +        let absval = Math.abs(event.delta);
> +        if (this.supportsRotation && absval >= 90 && absval < 180) {

Magic Numbers?

@@ +830,5 @@
> +        this.mMagnifyAmount = 0;
> +      ]]></handler>
> +      <handler event="MozMagnifyGestureUpdate"><![CDATA[
> +        if (this.supportsZoom) {
> +            const THRESHOLD = 30;

Constant should be declared somewhere else? No?

Comment 4

[Philipp Kewisch [:Fallen][☁️📆]]

(In reply to Mohit Kanwal [:redDragon] from comment #3)
> > +        if (this.supportsRotation && absval >= 90 && absval < 180) {
> Magic Numbers?
I'd like to accept rotation if rotation is between 90 and 180 degrees, as everything else is turning it around.

Not sure if a constant is right here, since it would just be:
const ROTATE_90_DEGREES = 90;

Maybe a comment? What do you think?

> 
> @@ +830,5 @@
> > +        this.mMagnifyAmount = 0;
> > +      ]]></handler>
> > +      <handler event="MozMagnifyGestureUpdate"><![CDATA[
> > +        if (this.supportsZoom) {
> > +            const THRESHOLD = 30;
> 
> Constant should be declared somewhere else? No?
Generally yes, but I have some issues with setting this directly on the binding. Bindings don't have a way to make a property constant, and also this threshold is not used anywhere else in the binding.

Therefore I thought it would be better to keep the constant close to where its used. I could put it one line up though.

(In reply to Mohit Kanwal [:redDragon] from comment #2)
> I tested it on my Mac ;) ! Th gestures work fine on 10.7 Lion. Swipe is the
> most responsive of the lot. Rotate seems a bit sluggish may be it is coz of
> the time and sudden nature of the animation when it swaps the time to the
> day in the view. May require a bit of tweaking. Usually I can rotate with
> one hand, and dont have to necessarily rotate it by 180 degrees.
Does it not rotate if more than 90 degrees? I agree its a bit sluggish, but our views aren't set up for the cool rotate-while-dragging transition. I guess this could be done using a canvas to screenshot the view, then use that screenshot while dragging, and morph to rotated view when dropped but that would be a lot of work :-)

> Zoom is also cool, rendering seems to be a bit slow though. I don't know if
> we can improve that.
Probably due to the reloading of the view, not sure we can improve without fixing other bugs first.

> Will there be a changelog dialog for Mac Users when this patch is pushed to
> the next release? Something like Skype [1]. 
We could put this in the release notes, but I doubt users will actually read them. If we had a "whats new" page, then maybe, but thats part of a different bug.


Let me know what of the above you think should be changed, then I'll fix this before checkin.

Comment 5

[Mohit Kanwal [:redDragon]]

(In reply to Philipp Kewisch [:Fallen] (away until May 28th) from comment #4)
> (In reply to Mohit Kanwal [:redDragon] from comment #3)
> > > +        if (this.supportsRotation && absval >= 90 && absval < 180) {
> > Magic Numbers?
> I'd like to accept rotation if rotation is between 90 and 180 degrees, as
> everything else is turning it around.
> 
> Not sure if a constant is right here, since it would just be:
> const ROTATE_90_DEGREES = 90;
> 
> Maybe a comment? What do you think?
> 
Yup a comment will do, I was more like thinking on the lines of this being a hardware related setting and therefore could be changed from some other method something like the scroll sensitivity in firefox that can be changed from the about:config url. 

> Generally yes, but I have some issues with setting this directly on the
> binding. Bindings don't have a way to make a property constant, and also
> this threshold is not used anywhere else in the binding.
> 
> Therefore I thought it would be better to keep the constant close to where
> its used. I could put it one line up though.
>
Yup I think one liner would be okay, at least locally we could keep the constants in one place.
 
> Does it not rotate if more than 90 degrees? I agree its a bit sluggish, but
> our views aren't set up for the cool rotate-while-dragging transition. I
> guess this could be done using a canvas to screenshot the view, then use
> that screenshot while dragging, and morph to rotated view when dropped but
> that would be a lot of work :-)
>
For rotation, usually in iPhoto if I put my right thumb at a constant place on the trackpad and then rotate the index finger of my hand, i can get the photo to rotate, but in the calendar view i had to put a bit more effort to make it exactly a replica of the rotation action, as if performed by 2 fingers.

> > Zoom is also cool, rendering seems to be a bit slow though. I don't know if
> > we can improve that.
> Probably due to the reloading of the view, not sure we can improve without
> fixing other bugs first.
> 
> > Will there be a changelog dialog for Mac Users when this patch is pushed to
> > the next release? Something like Skype [1]. 
> We could put this in the release notes, but I doubt users will actually read
> them. If we had a "whats new" page, then maybe, but thats part of a
> different bug.
> 
Yep, no one reads the release notes :) ! There should be a what's new page? Usually addons can load a whats new page when they first start up. Lightning does not have one? 
> 
> Let me know what of the above you think should be changed, then I'll fix
> this before checkin.

Comment 6

[Philipp Kewisch [:Fallen][☁️📆]]

(In reply to Mohit Kanwal [:redDragon] from comment #5)
> Yup a comment will do, I was more like thinking on the lines of this being a
> hardware related setting and therefore could be changed from some other
> method something like the scroll sensitivity in firefox that can be changed
> from the about:config url. 
It looks like firefox has "browser.gesture.twist.threshold" and a few others, but Thunderbird doesn't use any of those prefs, nor has any other prefs with "gesture" in them. I think we should stay with const values for now.

I've changed the values to minimum 45 degrees rotation, that should allow rotation with one hand, and I've added a comment.

> > > Zoom is also cool, rendering seems to be a bit slow though. I don't know if
> > > we can improve that.
> > Probably due to the reloading of the view, not sure we can improve without
> > fixing other bugs first.
Actually wrong, its due to the nature of the zoomIn commands. Zooming in adds exactly one hour, so we add one hour every 30 mouse zoom pixels. Changing the zoom will cause a small jump, since the display will add/remove one hour.

The only way to fix this would be allowing to zoom by minutes and not by hours, which doesn't really work out since the pref is an integer pref. This wouldn't be so bad since the function does actually set visible minutes, but the code is strange and refreshes day selection when the visible minutes are changed, so I wouldn't want to fix this just yet.

> > > Will there be a changelog dialog for Mac Users when this patch is pushed to
> > > the next release? Something like Skype [1]. 
> > We could put this in the release notes, but I doubt users will actually read
> > them. If we had a "whats new" page, then maybe, but thats part of a
> > different bug.
> > 
> Yep, no one reads the release notes :) ! There should be a what's new page?
> Usually addons can load a whats new page when they first start up. Lightning
> does not have one? 
Nope, Lightning does not have one. See bug 413641.

Comment 7

[Philipp Kewisch [:Fallen][☁️📆]]

Attachment: Fix - v2

Comment 8

[Mohit Kanwal [:redDragon]]

Comment on attachment 626308 [details] [diff] [review]
Fix - v2

Review of attachment 626308 [details] [diff] [review]:
-----------------------------------------------------------------

Awesome!! The rotation is quite funky now, managed to do it with one hand !!

Comment 9

[Philipp Kewisch [:Fallen][☁️📆]]

Pushed to comm-central changeset de1c514775e6