Week view and day view should scroll to useable time

VERIFIED FIXED in Sunbird 0.3

Status

enhancement
VERIFIED FIXED
17 years ago
11 years ago

People

(Reporter: gullc, Assigned: thomas.benisch)

Tracking

unspecified
Sunbird 0.3
x86
Windows 2000

Details

Attachments

(2 attachments, 1 obsolete attachment)

Reporter

Description

17 years ago
I'm using Mozilla 0.9.9 with the XPI of 22nd of March.

-Start up calender, go to daily/weekly view.
-what you see on a monitor with average resolution is the time from midnight
till nine AM. For the average user, who usually has his appointments from nine
till ten PM, this does not present a good overview.

My request: the scroll bar should be attached so that the day starts with the
earliest appointment, or at seven AM.

Comment 1

17 years ago
I could have swore we had a bug on this already.  I guess not.  Confirmed.  I
think the goal we actually have for this is to start at 8AM (when most people's
days start).
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

17 years ago
Summary: Week/Day window should be arranged differently → Week view and day view should scroll to useable time

Comment 2

17 years ago
Is bug 52140 a blocker for this bug? 
Or bug 85213?
Bottom line: I don't know how to scroll XUL elements with overflow: auto; on them.

Comment 3

17 years ago
Karl,

Can you think of anyway to do this?

Updated

17 years ago
Keywords: helpwanted

Comment 4

17 years ago
How about a pref to define the default start and end times of the day, and have
calendar attempt to show the times between (e.g., 8:00 to 18:00) - but at least
the start time (minus 1 hour?).

A possible solution to having Calendar scroll to a certain time could be by
using "anchors"!? (please don't laugh, I know next to nothing about HTML, let
alone XUL).

Comment 5

17 years ago
I keep thinking that one of the scroll methods of the DOM would work but I'm not
programming for the next two weeks. Mailnews does it so it has to be possible.
(The top scrollpane jumps to the next unread message when I hit 'n')

Plairo: a decent idea, but xul doesn't really support anchors in the same way
that html does, so it won't work.

Comment 6

17 years ago
The mechanism used for mail/news thread pane scrolling appears to be unworkable
for calendar day view scrolling.

MailNews thread pane scrolling is implemented with tree/listbox scrolling.

It is possible to use a listbox for the day view hours, then scroll to the
current hour.

HOWEVER, events may span multiple hours, and are shown by stacking a box that
spans the hours over the hour display.

If the events are over the entire list, they do not scroll with the list.  This
is unacceptable.

If the events are inside the list, each event must fit inside a list item.  In
other words, no events that span hours.  This seems unacceptable.

So it appears list/tree scrolling as the mailnews thread pane uses is not a
workable solution. 

Comment 7

17 years ago
I don't know how to do this.  Sending this to nobody@mozilla.org so others can
find it and they can work on it.
Assignee: mikep → nobody
Reporter

Comment 8

17 years ago
Scrolling elements with CSS overflow:auto or overflow:scroll on them doesn't
seem possible right now.

Bug #62536 blocks it, I guess. ([RFE] function/property to get scroll position
of elements)

Would it be possible to use no overflow but instead place the entire area inside
a XUL scrollbox? Then we could use something like ScrollbyIndex to scroll to the
first event box.

Updated

17 years ago
Target Milestone: --- → 1.0

Comment 9

17 years ago
*** Bug 170684 has been marked as a duplicate of this bug. ***

Comment 10

17 years ago
*** Bug 172609 has been marked as a duplicate of this bug. ***

Comment 11

17 years ago
This should not be done with scroll. I dont even want to see the hours from
midnight to morning. Unless offcourse I have an appointment...

Comment 12

17 years ago
->Mine
Assignee: nobody → mikep
Severity: trivial → enhancement

Updated

17 years ago
Status: NEW → ASSIGNED

Comment 13

16 years ago
New contact from mikep@oeone.com to mostafah@oeone.com
Filter on string OttawaMBA to get rid of these messages. 
Sorry for the spam.
Assignee: mikep → mostafah
Status: ASSIGNED → NEW
gekacheka:
Any ideas on this now that we're reworking the views?
Target Milestone: 1.0 → Sunbird 0.3
QA Contact: colint → sunbird
Reassigning all automatically assigned bugs from Mostafa to nobody@m.o

Bugspam filter: TorontoMostafaMove
Assignee: mostafah → nobody
Assignee

Comment 16

13 years ago
I will reuse this Bug as a follow-up of Bug 323093.
Assignee

Comment 17

13 years ago
I think the 24 hours feature includes the following steps
(not necessarily in this order):

1.) display 24 hours by default
2.) add preferences for working hours and display them in different colour
3.) scroll to usable time, e.g. first working hour or first event
4.) marker for out-of-sight-events
5.) remove autoexpand code

Please note, that there was already some discussion on this subject 
in Bug 323093. Nevertheless I would like to discuss some details:

1.) display 24 hours by default
    ---------------------------
I think it makes sense, that 24 hours are displayed always, that means
that the user cannot change the start or end time in the UI.
Otherwise we would have to keep the autoexpand feature and the related
code alive. 
As a consequence the preferences calendar.view.defaultstarthour and
calendar.view.defaultendhour can be removed. Probably somebody knows
if this is possible, or if there are any problems or incompatibilites
with existing user profiles.
In principal also all setStartEndMinutes methods can be removed.

2.) add preferences for working hours and display them in different colour
    ----------------------------------------------------------------------
For working hours we want to have the same features we have now for
start and end minutes, that means the user should be able to configure
the start and end working hours in the UI. Therefore we need some
preferences (calendar.view.deaultstartworkhour and
calendar.view.defaultendworkhour), fields mStartWorkMin and mEndWorkMin
and corresponding setters (setStartEndWorkMinutes).

3.) scroll to usable time, e.g. first working hour or first event
    -------------------------------------------------------------
Any preferences, if the earliest event of the day/week or the first
working hour is the right time to scroll to?

4.) and 5.) can be discussed later.
Assignee

Comment 18

13 years ago
taking over this task
Assignee: nobody → thomas.benisch

Comment 19

13 years ago
(In reply to comment #17)
> I think the 24 hours feature includes the following steps
> (not necessarily in this order):
> 
> 1.) display 24 hours by default
> 2.) add preferences for working hours and display them in different colour
> 3.) scroll to usable time, e.g. first working hour or first event
> 4.) marker for out-of-sight-events
> 5.) remove autoexpand code
This looks about right.  I'm not sure whether we want to explicitly call out the fact that our day-columns, as a result of showing 24hr views, should not build their xul dynamically.  That may just be an implementation detail, but it's something that should definitely come about as a result of these changes.  That will also make it easier to solve the regression about header-boxes being misaligned.

> 1.) display 24 hours by default
>     ---------------------------
> I think it makes sense, that 24 hours are displayed always, that means
> that the user cannot change the start or end time in the UI.
> Otherwise we would have to keep the autoexpand feature and the related
> code alive. 
> As a consequence the preferences calendar.view.defaultstarthour and
> calendar.view.defaultendhour can be removed. Probably somebody knows
> if this is possible, or if there are any problems or incompatibilites
> with existing user profiles.
> In principal also all setStartEndMinutes methods can be removed.
> 
Yes on all counts.
> 2.) add preferences for working hours and display them in different colour
>     ----------------------------------------------------------------------
> For working hours we want to have the same features we have now for
> start and end minutes, that means the user should be able to configure
> the start and end working hours in the UI. Therefore we need some
> preferences (calendar.view.deaultstartworkhour and
> calendar.view.defaultendworkhour), fields mStartWorkMin and mEndWorkMin
> and corresponding setters (setStartEndWorkMinutes).
There are several interesting questions related to these that I think are going to need to go through our UI process.  As such, I'd like to push these to the end of the workflow here, so that we don't get stalled.

> 
> 3.) scroll to usable time, e.g. first working hour or first event
>     -------------------------------------------------------------
> Any preferences, if the earliest event of the day/week or the first
> working hour is the right time to scroll to?
Again, there are interesting UI questions here.  I would probably be ok with, as a temporary fix, just going ahead and showing 8am as a default during the transition.  I'd want dmose and/or mvl to sign off on that decision before we landed it though.

As I see it, the path forward looks something like:
1.) Start showing all 24hrs.  At its simplest, this is a trivial fix.  Along with it, we can probably yank all the recalculation stuff in this patch too.
2.) Handle auto-scrolling to 8am.  (This includes getting UI signoff.)  If this is less than 10 lines (I don't remember how much scrollbox hacking we had to do.) we can include it above.  If not, it should be its own patch, but should land with (1).
3.) Bring up the workhours and out-of-view indicator questions for UI discussion.
3a.) Write/land the workhours patch.
3b.) Write/land the out-of-views patch.

All of (3) probably wants to wait until after 0.3, but I could be persuaded otherwise.  I think we definitely want (1) and (2) sooner rather than later.
Assignee

Comment 20

13 years ago
(In reply to comment #19)
> (In reply to comment #17)
> > I think the 24 hours feature includes the following steps
> > (not necessarily in this order):
> > 
> > 1.) display 24 hours by default
> > 2.) add preferences for working hours and display them in different colour
> > 3.) scroll to usable time, e.g. first working hour or first event
> > 4.) marker for out-of-sight-events
> > 5.) remove autoexpand code
> This looks about right.  I'm not sure whether we want to explicitly call out
> the fact that our day-columns, as a result of showing 24hr views, should not
> build their xul dynamically.  That may just be an implementation detail, but
> it's something that should definitely come about as a result of these changes. 
> That will also make it easier to solve the regression about header-boxes being
> misaligned.

That is really a good point, which I didn't have in mind until now. I think
it's worth to add 'build day columns statically' as an additional item.
As you already wrote, switching to 24 hours can be implemented very simple
in a first step or rather advanced (remove autoexpand code, build day columns
statically). The question is, if the first patch is rather simple or advanced.
Apart from that I don't understand the correlelation to the misaligned header
boxes. I think that's an independent problem.

> > 1.) display 24 hours by default
> >     ---------------------------
> > I think it makes sense, that 24 hours are displayed always, that means
> > that the user cannot change the start or end time in the UI.
> > Otherwise we would have to keep the autoexpand feature and the related
> > code alive. 
> > As a consequence the preferences calendar.view.defaultstarthour and
> > calendar.view.defaultendhour can be removed. Probably somebody knows
> > if this is possible, or if there are any problems or incompatibilites
> > with existing user profiles.
> > In principal also all setStartEndMinutes methods can be removed.
> > 
> Yes on all counts.
> > 2.) add preferences for working hours and display them in different colour
> >     ----------------------------------------------------------------------
> > For working hours we want to have the same features we have now for
> > start and end minutes, that means the user should be able to configure
> > the start and end working hours in the UI. Therefore we need some
> > preferences (calendar.view.deaultstartworkhour and
> > calendar.view.defaultendworkhour), fields mStartWorkMin and mEndWorkMin
> > and corresponding setters (setStartEndWorkMinutes).
> There are several interesting questions related to these that I think are going
> to need to go through our UI process.  As such, I'd like to push these to the
> end of the workflow here, so that we don't get stalled.

I think from a feature point of view working hours can be shifted at the end
of the workflow. Nevertheless there are some correlations with switching to 24 hours.
Removing the preferences and setStartEndMinutes method and adding them later
for working hours with a different name is only some kind of renaming, and
from an implementation point of view simpler to do in one patch.
But that's not really an argument. Much more important are the UI related issues.
For 24 hours we must in principal remove the UI fields for start and end time
from Tools/Options/(Lightning/)Views and add them later again for working hours.
Also here reusing and renaming would be much simpler.
So the question is, do we want to modify the UI already for the 24 hours patch? 

> > 3.) scroll to usable time, e.g. first working hour or first event
> >     -------------------------------------------------------------
> > Any preferences, if the earliest event of the day/week or the first
> > working hour is the right time to scroll to?
> Again, there are interesting UI questions here.  I would probably be ok with,
> as a temporary fix, just going ahead and showing 8am as a default during the
> transition.  I'd want dmose and/or mvl to sign off on that decision before we
> landed it though.
> 
> As I see it, the path forward looks something like:
> 1.) Start showing all 24hrs.  At its simplest, this is a trivial fix.  Along
> with it, we can probably yank all the recalculation stuff in this patch too.
> 2.) Handle auto-scrolling to 8am.  (This includes getting UI signoff.)  If this
> is less than 10 lines (I don't remember how much scrollbox hacking we had to
> do.) we can include it above.  If not, it should be its own patch, but should
> land with (1).
> 3.) Bring up the workhours and out-of-view indicator questions for UI
> discussion.
> 3a.) Write/land the workhours patch.
> 3b.) Write/land the out-of-views patch.
> 
> All of (3) probably wants to wait until after 0.3, but I could be persuaded
> otherwise.  I think we definitely want (1) and (2) sooner rather than later.

Actually I wanted to have also (3) finished for 0.3, but let's see how (1) and
(2) moves forward.
Assignee

Comment 21

13 years ago
Posted patch patch v1 (obsolete) — Splinter Review
This patch enables 24 hours view by default.

NOTE: In addition to this patch the file
      mozilla/calendar/base/content/preferences/views.js
      can be removed. I didn't know how to include that
      in the patch.

In the day and week view 24 hours are displayed by default. Initially the view
scrolls to 8 a.m., this will be changed as soon as working hours are available.
As agreed, I removed the preferences calendar.view.defaultstarthour and
calendar.view.defaultendhour, and all the corresponding UI in the Tools/Options
dialogs for Lightning and SunBird.

In addition I removed all the autoexpand code, which is a significant 
performance win. Note, that this patch doesn't include to build the day columns
statically, but this will be the next step in another patch.

Some more technical details:
I got problems with the resize handler (onResize) with 24 hours, therefore
I changed there a lot. Instead of one minimum value for pixels per minute
we now have 2, one for horizontal, one for vertical orientation.
The values are mMinHorizontalPixelsPerMinute = 1.5 and
mMinVerticalPixelsPerMinute = 0.6. Please check, if you're fine with those
values. You once had problems with a value of 0.6 for vertical orientation,
but I think this was caused by a bug in the resize handler.
A value of 0.6 corresponds to 36 pixels per hour, which gives 288 pixels
for 8 hours. So this should be fine.
Attachment #232887 - Flags: first-review?(jminta)
Assignee

Updated

13 years ago
Whiteboard: [cal-ui-review needed]

Comment 22

13 years ago
Comment on attachment 232887 [details] [diff] [review]
patch v1

-      <field name="mMinPPM">0.4</field>
+      <field name="mMinHorizontalPixelsPerMinute">1.5</field>
+      <field name="mMinVerticalPixelsPerMinute">0.6</field>
This does still feel too big for me.  I think .4 (and maybe 1 or 1.2) was the right number.

+          var minute;
Can you make this var minute = 0 so that we don't run the risk of returning null here?  (JS will almost always do the right thing with null, but still...)

-<!ENTITY time.midnight "Midnight" >
-<!ENTITY time.1 "1:00 AM" >
-<!ENTITY time.2 "2:00 AM" >
-<!ENTITY time.3 "3:00 AM" >
I'd say don't remove these entities.  They're likely useful elsewhere.

r=jminta with that!
Attachment #232887 - Flags: first-review?(jminta) → first-review+

Updated

13 years ago
Whiteboard: [cal-ui-review needed] → [cal-ui-review needed][high risk]
Assignee

Comment 23

13 years ago
(In reply to comment #22)
> (From update of attachment 232887 [details] [diff] [review] [edit])
> -      <field name="mMinPPM">0.4</field>
> +      <field name="mMinHorizontalPixelsPerMinute">1.5</field>
> +      <field name="mMinVerticalPixelsPerMinute">0.6</field>
> This does still feel too big for me.  I think .4 (and maybe 1 or 1.2) was the
> right number.

With those numbers the boxes are really small. What screen resolution do
you have? Nevertheless I changed those numbers to 1.0 and 0.4. 

> +          var minute;
> Can you make this var minute = 0 so that we don't run the risk of returning
> null here?  (JS will almost always do the right thing with null, but still...)

done

> -<!ENTITY time.midnight "Midnight" >
> -<!ENTITY time.1 "1:00 AM" >
> -<!ENTITY time.2 "2:00 AM" >
> -<!ENTITY time.3 "3:00 AM" >
> I'd say don't remove these entities.  They're likely useful elsewhere.

done
Assignee

Comment 24

13 years ago
Posted patch patch v2Splinter Review
This patch fixes all remarks from comment #22.
I carried over the r+ flag from the last patch.

I guess cal-ui-review is still pending?
Are there any further actions required from my side?
After the ui issue is resolved, can you please land
this patch? Thanks.
Attachment #232887 - Attachment is obsolete: true
Attachment #233559 - Flags: first-review+
Whiteboard: [cal-ui-review needed][high risk] → [cal-ui-review needed][high risk][patch in hand]
Whiteboard: [cal-ui-review needed][high risk][patch in hand] → [ui-review+][high risk][patch in hand]

Comment 25

13 years ago
Patch checked in.  Please file follow-up bugs (or find previously filed ones) for additional tweaks/changes.
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Assignee

Comment 26

13 years ago
Posted image Lightning week view
Lightning week view with mMinVerticalPixelsPerMinute = 0.4
Assignee

Comment 27

13 years ago
This is really great that you managed to review the patch and gave
ui-review+ in time, so that this will be part of the 0.3 release.

Nevertheless I think with the current values for 
mMinHorizontalPixelsPerMinute and mMinVerticalPixelsPerMinute
there's another small problem which needs some fine-tuning.

Due to the small value of 0.4 for mMinVerticalPixelsPerMinute basically 
24 hours are visible in the week view in Lightning. See also the
screenshot from comment #26 (My screen resolution is 1280x1024.).
The scrollbar only appears, if I resize the window to a much smaller
size. The problem is the unused space at the bottom of the week view.

The reason for this space is due to the fact, that pixelsPerMinute is
rounded to a precision of 0.1. This makes a precision of 6 pixels
(60 minutes x 0.1 ppm) per box or 144 pixels for 24 hours.
That means, that the boxes are only resized to the next bigger size,
when the visible area increases by 144 pixels.
In principal we need only a precision of 1 pixel per box, that means
24 pixels for 24 hours.

Therefore I suggest the following solution for the mentioned problem:
1. In the onResize method don't round pixelsPerMinute to a precision
   of 0.1, but to much higher precisions, e.g. 0.001 or don't round at
   all.
2. In all places, where mPixPerMin is used to calculate the size of a
   box, e.g. makeTimeBox(timeString, dur * this.mPixPerMin) round
   the pixels: makeTimeBox(timeString, Math.floor(dur * this.mPixPerMin)).

Any comments?
(In reply to comment #27)
> Due to the small value of 0.4 for mMinVerticalPixelsPerMinute basically 
> 24 hours are visible in the week view in Lightning. See also the
> screenshot from comment #26 (My screen resolution is 1280x1024.).
> The scrollbar only appears, if I resize the window to a much smaller
> size. The problem is the unused space at the bottom of the week view.
> 

Agreed.  I suggest spinning that problem off to a new bug.  I think Joey has his head around that code better than I do, so I'm gonna leave the commenting about the best fix to that problem to him.

That said, on an SXGA screen (1024x768, a common size for laptop screens), 0.4 is unworkably small.  It's impossible to read text in half-hour events, and it's impossible to move 15 minute events by dragging because there's no place between the resize bars to grab.  We're going to need at least 0.5 here; cal-ui-review+ and rs=dmose (rubberstamp) for a patch that makes that change.  After that lands, we can collect feedback to decide if we think 0.6 is a better value for 0.3.

For post-0.3, let's file another bug as well to look at the higher level design problem.  There are a bunch of interesting constraints here including screensize, screen DPI, CSS pixel -> device-pixel mapping (I know hyatt has blogged about this some in the Safari blog), font size, box behavior for small events, etc.

Comment 29

13 years ago
(In reply to comment #28)
> For post-0.3, let's file another bug as well to look at the higher level design
> problem.  There are a bunch of interesting constraints here including
> screensize, screen DPI, CSS pixel -> device-pixel mapping (I know hyatt has
> blogged about this some in the Safari blog), font size, box behavior for small
> events, etc.
> 
I don't think any of the things listed here are really the important issues.  The important question is the way that people use these views.  For large amounts of manipulation/fine-grained control, the larger values are ideal.  For overviews/at-a-glance summary, the smaller numbers are better.  (The views with larger pixel values here constantly frustrate me because I can never see more than half of the time I care about.  The time from 6pm-12pm is just as important in my schedule as 'normal working hours.')  We need to come up with a solution that addresses both use-cases.
Sure, those were implementation details.  The spinoff bug should be about the larger design problem, which is, as you rightly point out, the use cases.
Checked in a patch bumping the minimum vertical pixels to 0.5.
(In reply to comment #31)
> Checked in a patch bumping the minimum vertical pixels to 0.5.

Please also check in to MOZILLA_1_8_BRANCH; so that trunk and branch are in sync again.
Assignee

Comment 33

13 years ago
(In reply to comment #31)
> Checked in a patch bumping the minimum vertical pixels to 0.5.

Thanks for this patch, I had a day off.
Assignee

Comment 34

13 years ago
(In reply to comment #28)
> Agreed.  I suggest spinning that problem off to a new bug.

I filed Bug #349509.
Assignee

Comment 35

13 years ago
(In reply to comment #25)
> Patch checked in.  Please file follow-up bugs (or find previously filed ones)
> for additional tweaks/changes.

I filed Bug #349518 for out-of-sight events and Bug #349520 for working hours.

Comment 36

13 years ago
there were many changes from that time, however let me verified it with one of previous version

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060816 Calendar/0.3a2+
Status: RESOLVED → VERIFIED
Keywords: helpwanted
Whiteboard: [ui-review+][high risk][patch in hand]
You need to log in before you can comment on or make changes to this bug.