Closed Bug 375389 Opened 17 years ago Closed 17 years ago

Allow mouse wheel scrolling in rotated view

Categories

(Calendar :: Calendar Frontend, defect)

defect
Not set
minor

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: Fallen, Assigned: Fallen)

Details

Attachments

(1 file, 1 obsolete file)

Currently, when you rotate the view you loose the functionality of being able to scroll with the mouse wheel. This works in the non-rotated view.
I'd propose to get rid of the horizontal view. This is a totally useless piece of functionality that just causes additional complexity without any gain in usability.
I personally haven't used it either, but I can see the use of it. There have been discussions on the rotated view's necessity. See bug 358692, which even contains a screenshot to why this view may be needed.

If we do have this view, we should make sure its usable.
Attached patch Add Scroll functionality - v1 (obsolete) — — Splinter Review
This patch allows using the mouse wheel for scrolling in both view orientations. 
* The multiday views scroll by one hour per wheel move
* multiweek views scroll one month/week per wheel move
Assignee: nobody → bugzilla
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #267858 - Flags: review?(michael.buettner)
Comment on attachment 267858 [details] [diff] [review]
Add Scroll functionality - v1

>+          // Scroll by exactly one hour regardless of which orientation we have
>+          if (childbox.getAttribute("orient") == "horizontal") {
>+            scrollBoxObject.scrollBy(0, (event.detail < 0 ? -1 : 1) * this.mPixPerMin * 60);
>+          } else {
>+            scrollBoxObject.scrollBy((event.detail < 0 ? -1 : 1) * this.mPixPerMin * 60, 0);
>+          }
I'd write something like:
  var offset = (event.detail < 0 ? -1 : 1) * this.mPixPerMin * 60;
  if (orientation == horizontal) {
    scrollBoxObject.scrollBy(0, offset);
  } else {
    scrollBoxObject.scrollBy(offset, 0);
  }
But that's just nitpicking, address it if you're inclined.

The code itself works as advertised. What bogs me is the fact that with this patch the views flip to the previous/next timeslot, except the week-view. The scrolling in the week-view is restricted to the horizontal/vertical movement within the view itself. Even worse, if you configure the week-view to display the full 24 hour range, the mouse wheel does nothing (of course). I'd like to have Christian throw in a comment on that before giving r+.
Attachment #267858 - Flags: ui-review?(christian.jansen)
From my point of view it feels somehow strange that an up/down direction moves something to left/right. Scrolling up & down by using the scroll wheel should not shift the hours, but the days. 

For the "standard" views I would expect the following mouse wheel scrolling behavior:

Day View:
As implemented

Week View:
As Implemented

Multi Week
Up -> scrolls to previous week
Down -> scrolls to next week


Month View

Up -> scrolls to previous week
Down -> scrolls to next week

Additionally Lightning should support the "AutoScroll" navigation feature (Long Middle Mouse Button/Scroll wheel click), but I guess this would be another task ;-)
But this means, that the mouse wheel has a new meaning in the rotated views. It might also seem strange, if in normal view the mouse wheel scrolls down the hours, and in rotated view it scrolls to a different day, even though there is a  scrollbar left-right.

In month view, do you really mean weeks?

What we could do, is use a modifier like Ctrl. I.e normal scrolling as in my patch and using Ctrl+wheel to switch days/weeks in the day/week views. Since the month/multiweek view has no scrollbar, I think it is ok to change months with both ctrl+wheel and wheel. Short overview:

Day/Week View:
Ctrl+Wheel: move weeks/days up/down (standard and rotated)
Wheel: move hours up/down (standard) or left/right (rotated)

Month View:
Ctrl+Wheel: move moths/weeks up/down
Wheel: move moths/weeks up/down


I think the long scrolling should work too, doesn't that just send multiple scroll events?
I have the feeling that this is the beginning of a sea rose. So I'll stop here and we can do the big thing after 1.0.

We should do what's in the patch for 1.0 plus what you mentioned in Comment #6

> Day/Week View:
> Ctrl+Wheel: move weeks/days up/down (standard and rotated)
> Wheel: move hours up/down (standard) or left/right (rotated)

> Month View:
> Ctrl+Wheel: move moths/weeks up/down
> Wheel: move moths/weeks up/down

After 1.0 we should think about a convinient week scrolling feature for the month view. This would make life easier for users who needs to schedule events/tasks at the boundaries of a month eg May-June.
(In reply to comment #6)
> Day/Week View:
> Ctrl+Wheel: move weeks/days up/down (standard and rotated)

I would not use Ctrl+Wheel. As far as I know this is often used as a zoom feature, e.g. to increase text size in Firefox. I could think of a similar usage in Sunbird/Lightning to e.g. increase the 'total hours in view' setting on the fly - similar to zooming in/out the calendar view. (But that would be a different bug too.) Maybe use Shift+Scroll?

A different solution might to add the "move weeks/days up/down" functionality to the navigation bar on top of the calendar view. If you scroll while the mouse is over this area you change the day/week, while the mouse is over the calendar view you scroll the hours.

(In reply to comment #7)
> After 1.0 we should think about a convinient week scrolling feature for the
> month view. This would make life easier for users who needs to schedule
> events/tasks at the boundaries of a month eg May-June.

This is already available with the Multiweek view. In Lightning it's currently only available via a separate extension but might be shipped by default in future.
Attached patch Add Scroll functionality - v2 — — Splinter Review
I agree, shift+Wheel is a better solution. This patch makes it work. I finally found out why I wasn't getting any DOMMouseScroll events for Ctrl and Shift, its a preference that has to be set differently. To make sure this is set correctly for lightning, I added some default prefs also for lightning.

To make it easier to transition to post-1.0 scrolling, I made sure that the said scroll actions ONLY happen if the shift modifier is pressed or no modifier at all is used. This way users can only complain about one key not working anymore for scrolling ;)

Another thing I wasn't too sure about but I think can also be postponed is that scrolling past the end of the day/week does not move to the next day. I think this is minor or even undesirable though, shift+Wheel can be used in such cases.
Attachment #267858 - Attachment is obsolete: true
Attachment #271021 - Flags: ui-review+
Attachment #271021 - Flags: review?(michael.buettner)
Attachment #267858 - Flags: ui-review?(christian.jansen)
Attachment #267858 - Flags: review?(michael.buettner)
Comment on attachment 271021 [details] [diff] [review]
Add Scroll functionality - v2

>+          <handler event="keypress">
>+            <![CDATA[
>+              const kKE = Components.interfaces.nsIDOMKeyEvent;
>+              switch (event.keyCode) {
>+                  case kKE.DOM_VK_PAGE_UP:
>+                      this.moveView(-1);
>+                      break;
>+                  case kKE.DOM_VK_PAGE_DOWN:
>+                      this.moveView(1);
>+                      break;
>+              }
CDATA-tags (opening and closing) should be placed on the same line as the enclosing tag. This  applies here as well as to the other parts of this patch.

>+                    this.moveView((event.detail < 0 ? -1 : 1));
>+                    this.moveView((event.detail < 0 ? -1 : 1));
>+                    this.moveView((event.detail < 0 ? -1 : 1));
There's a superfluous pair of brackets in each of these statement.

Since these are just style nits -> r=mickey.
Attachment #271021 - Flags: review?(michael.buettner) → review+
Checked in on HEAD and MOZILLA_1_8_BRANCH.

-> FIXED
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
OS: Windows XP → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → 0.7
Now I can use the mousewheel in month view!  I think that this is one of the greatest new features of 0.7.

It seems that you might have also fixed bug 241040.
Verified in lightning (build 2007080603) and sunbird (build 20070806) -> task is fixed.
Status: RESOLVED → VERIFIED
Flags: in-litmus?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: