Display Today Pane with Task List in Calendar mode too

VERIFIED FIXED in 0.9

Status

defect
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: ssitter, Assigned: berend.cornelius09)

Tracking

(Blocks 1 bug)

unspecified
Dependency tree / graph
Bug Flags:
wanted-calendar0.9 +

Details

Attachments

(1 attachment, 12 obsolete attachments)

110.01 KB, patch
berend.cornelius09
: review-
chris.j.bugzilla
: ui-review+
Details | Diff | Splinter Review
Reporter

Description

12 years ago
Using Lightning 0.7pre (20070721) with Thunderbird 2.0.0.5pre (20070721)

Follow up for Bug 385900 Comment #30 and following comments. 

In my opinion it should be possible to enable the Today Pane also in Calendar Mode to quickly view all calendar related information like agenda, tasks, events and calendars in one place. 

Currently you need to switch between Mail Mode and Calendar Mode again and again just to view your calendar information.

One use case from Bug 385900 Comment #36:
Due to the new toolbar switching feature the New Task button is only available in Calendar Mode. To view your other tasks during creation or to view the new created tasks you need to switch back to Mail Mode.

Another use case from me:
I use a lot of calendars to organize my events and tasks. By default only a view of them are enabled and visible. From time to time I enable this calendars to check the events and tasks on them. Now this includes switching to Calendar Mode to enable the calendar, switch back to Mail Mode to view Agenda and Task list for this calendar, switch back to Calendar Mode to disable the calendar. This should be possible from within Calendar Mode only.

Comment 1

12 years ago
We should not discard what we can learn from Outlook 2007, which features a dramatically improved calendar comparing to earlier versions, and is the obvious inspiration for Lightning's Today Pane anyway.

Observations:
The right sidebar ("To-Do Bar", comparable to the Today Pane), is hidden in Calendar mode. In email mode you create a task by just typing it in the provided Edit control.
http://blogs.msdn.com/outlook/archive/2007/06/21/introducing-the-to-do-bar.aspx

In the week and day views tasks are instead shown in a separate area below each day. Tasks can be dragged from there on the day's calendar - to block time to complete the task (important for the free/busy grid).
http://www.winsupersite.com/showcase/office2007_screenshot_wrapper_01.asp?img=/images/showcase/office2007_rtm_10.jpg
I'm not sure how tasks are shown in month view, though.

Generally I'm hesitant showing the Today pane in calendar mode, as space is limited and days (especially in month/multiweek views) would shrink below a usable level.
Reporter

Comment 2

12 years ago
(In reply to comment #1)
First, I don't care what Outlook does.

Second, the Today Pane is not always shown. You can show or hide it in Mail Mode. I request the same functionality for the Calendar Mode. If you don't need it you can hide it. I need it but I can't show it currently.

Comment 3

12 years ago
(In reply to comment #2)
> (In reply to comment #1)
> First, I don't care what Outlook does.

Are you saying you don't evaluate the competition, but design calendar by pure intuition?

> Second, the Today Pane is not always shown. You can show or hide it in Mail
> Mode. I request the same functionality for the Calendar Mode. If you don't need
> it you can hide it. I need it but I can't show it currently.

What would it take to enable the pane, is it just that the toolbar button is registered for the calendar mode customization palette?

To support my wish, there'd need to be separate "collapsed" states for the Today Pane, so I can have it open in Mail, but closed in Calendar/Task modes.

Comment 4

12 years ago
I would prefer to have agenda and task list in calendar mode within the left pane  together with my calendar list. In mail mode, an option would be nice whether the today pane is shown in an extra sidebar or in the lower part of the folder pane.

Comment 5

12 years ago
(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > First, I don't care what Outlook does.
> 
> Are you saying you don't evaluate the competition, but design calendar by pure
> intuition?
> 
> > Second, the Today Pane is not always shown. You can show or hide it in Mail
> > Mode. I request the same functionality for the Calendar Mode. If you don't need
> > it you can hide it. I need it but I can't show it currently.
> 
> What would it take to enable the pane, is it just that the toolbar button is
> registered for the calendar mode customization palette?
> 
> To support my wish, there'd need to be separate "collapsed" states for the
> Today Pane, so I can have it open in Mail, but closed in Calendar/Task modes.
> 

It is an interesting approach to display the today pane in calendar mode, but I'm not sure if the today pane should look exactly same in calendar mode. It might make sense to rearrange parts of it. I'll publish some ideas in the next couple of days. It would be great to get your feedback on that. For the transition period I vote for displaying the following items in the calendar mode:

* The views ;-)
* The mini month,
* below of that two tabs
  * one tab for the calendar list,
  * the other for Tasks.

Comment 6

12 years ago
(In reply to comment #4)

[...]
In mail mode, an option would be nice
> whether the today pane is shown in an extra sidebar or in the lower part of the
> folder pane.
> 
We received tons of negative feedback on displaying the stuff below the folder pane. That was one of the main reasons for doing the redesign.

Comment 7

12 years ago
(In reply to comment #4)
> I would prefer to have agenda and task list in calendar mode within the left
> pane  together with my calendar list. In mail mode, an option would be nice

I agree with Christian, it's always most usable and satisfying to have options to show or hide elements!

> whether the today pane is shown in an extra sidebar or in the lower part of the
> folder pane.
> 

I do not keep it secret any longer: I'm no friend of the new layout - but maybe "that's life" ;-)
I mostly liked two things: the permanent visible Minimonth (this kept me from attaching those ugly wall calendars) and the ability to access the main features (have a quick look at Todos, create Events etc.)

Indeed it was consuming much space, but there could be much better solutions - but this is not the right place to discuss.

I would encourage you all to think about a fully customizable ltnSidebar, today-pane and toolbar, if this is possible.
CCing to Mickey because he did much of the stuff for the new layout.

Comment 8

12 years ago
Instead of having a tab that's only for tasks, I think that it would be more useful if this tab showed both events and tasks.  The reason is that you could be working on a different day/week/month in the main view and it would be nice to be able to always see today's upcoming events & tasks in the today pane.  Plus, the today pane would look the same in calendar mode as it does in mail mode (except for the calendar list).

In mail mode, there's a thing at the top that lets you choose if you want to see events, tasks, or both.  Maybe we could have the same feature in calendar mode.  It could default to only show tasks, as Christian wants.

I also wonder about what happens in the today pane as you navigate to a different month in the main view.  In the current nightlies, changing the main view also changes the minimonth.  If the today pane were there, I assume that selecting a different date in the main view would also change the events/tasks in the today pane.  This would happen even when you click on the arrows in the main view to go to different months, and in this case the information in the today pane would be meaningless because the same day number (1-31) is always selected in the main view as you go from month to month.

I'm thinking that it would be better if the minimonth and the today pane always showed today's info except in two cases:  (1) you change the date in the minimonth, or (2) you do something in the main view to indicate that you want the minimonth and today pane to show the selected date of the main view.

For (2), this command could be on the toolbar or in the context menu of a day box.  And/Or we could take advantage of the day number (1-31) that's displayed in each day box.  A single click could change the minimonth and today pane to that date.  Another single click could return them to today.  A double click could switch the main view to day view.  I guess that I'm biased in my comments because I always use the month view and of course the day number is specific to this view.

Regardless of how you tell Lightning to do this synchronization, Lightning could automatically switch the today pane from the calendar list to the event/task lists if necessary.

Comment 9

12 years ago
I also think that a simpler approach would be okay.  It would keep the minimonth, the today pane, and the view in sync most of the time, without any need for the user to manually sync them.

Currently in the minimonth, you can change to different months and years, and nothing happens in the view until you click on a day in the minimonth.  Likewise, this approach could allow the user to change to different time periods in the view (using the arrow buttons, the page up/down keys, or the mouse wheel), and the today pane is not updated until the user clicks in the grid (e.g. clicks on a day box in month view).

Like the approach in my previous post, this would prevent the today pane from being needlessly updated as you jump from month to month in the view (and thus navigation is faster).  The disadvantage of this approach is that the today pane would only show today's info if you clicked on today in the view.
Duplicate of this bug: 395359
I believe most users will find it more convenient to have more space to read
and manage their emails (especially on smaller laptop monitors!) and have
little use for the list of installed calendars that is currently shown in the calendar view.

I would welcome the following changes:
1. Move the list of installed calendars to a configuration dialog. IMHO this is
something a user adjusts once and very rarely touches again.
2. Use the free space in the calendar view to host the today pane. (Of course giving the user the option to choose where he wants to see the today
pane would be even better.) :-)
3. Possibility to switch off minimonth for those that do not use it.

Comment 12

12 years ago
(In reply to comment #11)
I agree that nearly the complete sidebar is somehow wasted for the calendar list in calendar mode.

Why not integrating the calendar list into the today pane (like in older versions where it was displayed in TB's folder pane) and displaying this today pane automatically in calendar mode and if desired in mail mode (but then without calendar list)?
While I personally regard the calendar list as a configuration matter that unnecessarily clutters up every-day working space, I can imagine that other users make use of it daily. - Though I cannot see how...

For me the "perfect" solution is that which gives the user the freedom to decide on interface. I know this is only partially realizable in practise. 

The suggestion Thorsten made seems to be a practicable compromise between the positions I have seen in this thread. I welcome to see this implemented.

Comment 14

12 years ago
(In reply to comment #12)
> (In reply to comment #11)
> I agree that nearly the complete sidebar is somehow wasted for the calendar
> list in calendar mode.
> 
I agree with that completely. Currently I have in calendar view on the pane to the left: the mini month, today's date, my list of Calendars (3), and about 4 inches of white space. Looks to me like that real estate would be perfectly suited for task/event list.

I would like to mention that we discussed on the Hamburg F2F meeting at great length about how to perform invitations in the new UI, see [1]. We decided that there will be an invitation management UI that will be part of the "calendar" mode, see [2]. Although it may seem that there's a hell of a lot of wasted space below the list of calendars, this will be filled out by the upcoming invitation part.

[1] http://wiki.mozilla.org/Calendar:Hamburg_F2F_Meeting#Day_3_Monday
[2] http://wiki.mozilla.org/Calendar:Improving_the_Calendar_Views#Calendar
I appreciate that there is discussion on this topic. However, I maintain, most people will foremost want their tasklist in their calendar view - above anything else. I personally look at my task list several douzen times a day while I send out or receive just a few invitations per day. Filling the "free" space with a invitations UI would thus not address the "grievances" of this bug report.

How about reintroducing the tabs in the today pane and adding a tab for the invitations UI? This would be a much more efficient use of space. - And again give the user the chance to decide how their calendar should look like. Another possibility would be a "toggle view" button in the lower left-hand corner of the "free space" to toggle between tasks and invitations UI. (As we already have for calendar and mail view.)

Comment 17

12 years ago
I'll take the "I want to see my tasks in calendar mode"  requirement really serious, but I'd like to think about this topic in a more holistic way. Planning of the Task-Mode will start soon. This includes of coarse optimizing the Modes(Calendar, Mail & Task) & the Task dialog itself.

If you have special RFEs, or idea or what ever regarding Task please add them to the wiki page. Thanks.

http://wiki.mozilla.org/Calendar:Task_Mode

Comment 18

12 years ago
 Although it may seem that there's a hell of a lot of wasted
> space below the list of calendars, this will be filled out by the upcoming
> invitation part.
> 
I do realize that quite a bit of thought went into the current design and I never meant to imply otherwise. 
For a vote of tasks vs. calendar list vs. invites I definitely find Tasks more useful than anything. Ideally, however, it would be nice to have all three in the "free space" and divide them with sliding frames (not sure if that's the correct term), just like items and tasks are separated in the mail view. That would give everyone the option to easily customize to their liking.
Duplicate of this bug: 399423

Comment 20

12 years ago
I have this issue as well. I would like to see a task pane in calendar mode too.

While I see there is some discussion on this, I can't help but think the omission of a task list in calendar mode is inconsistent with the UI design of Sunbird 0.7.

The calendar view in Lightning 0.7 has plenty of real estate available for a task list as the left column is mostly the list of subscribed calendars which, certainly in my personal case, is mostly empty space. Therefore, there is much space available to display tasks, just as there is in Sunbird.

Having said all that, I am otherwise very happy with Lightning 0.7 and the addition of the today pane as an option in mail mode is a good idea imho.

However, I agree with replies #2 and #4 on this topic.

Updated

12 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee

Updated

12 years ago
Blocks: 401355

Comment 21

12 years ago
Totally agreed with request to have ToDos also with the calendar as it's with mail.
What is a calendar for NOT show things for the day???

It's only making 'dates' but also to handle 'ToDos'

How about blocking for 0.8 release?

Comment 22

12 years ago
I think this is a good resolution to this bug, but there are still two issues:

- With a clean profile the Today-Pane is shown in all three modes. If this behaviour is not intended, it would be easiest to rename the attributes 'collapsedin[Calendar|Task]Mode' to 'openin[Calendar|Task]Mode' and to adjust the code.

- I chose not to add a toolbar button to show/hide the Today-Pane; this is only possible through menu_View or the Splitter. The reason for this is, that a totally copied toolbar button produces only overhead. I think it would be better to have 'dual-mode' buttons, but I will cover this in another bug...
Attachment #299148 - Flags: ui-review?(christian.jansen)
Attachment #299148 - Flags: review?(Berend.Cornelius)
If we decide to hook up the today pane in calendar mode and or task mode as well, then we should make sure, that we hook it up in Sunbird at the same time.
Assignee: nobody → giermann
Version: Trunk → unspecified
Status: NEW → ASSIGNED

Comment 24

12 years ago
@Simon: I also thought about that.
But since the Today-Pane is very hard-wired to Lightning, I would suggest to file a spin-off bug for that.
It would really blow up code, since I found many references from inside 'today-pane.js' to element that only exist in Lightning. I started to work-around these by simple checks, whether the nodes exist...

Another point is, that the sidebar in Sunbird looks totally different and does already contain the tasks list. It would be a question to the UI team (Christian), whether and if, in what way the Today-Pane should be shown in Sunbird.
(In reply to comment #24)
> It would really blow up code, since I found many references from inside
> 'today-pane.js' to element that only exist in Lightning. I started to
> work-around these by simple checks, whether the nodes exist...
Please don't add more hardwired code. I'm strictly against that. It's already bad that the Today Pane directly references Lightning code. The correct solution is to make the agenda an overlay (just as the unifinder todo is) and make the today pane fire events in order to control these contained elements. The Today Pane should be a generic container that is able to host a number of elements, nothing more. Any further logic that is contained in the Today Pane is superfluous and just plain wrong. I can't make the final decision, but this is nevertheless something I just can't resist to complain about.

Comment 26

12 years ago
Comment on attachment 299148 [details] [diff] [review]
Show the Today-Pane also in Calendar and Task modes (all independently collapsable)

(In reply to comment #25)
> Please don't add more hardwired code. I'm strictly against that.
Yes, you are totally right. Maybe I will find a way to separate the Lightning code into Lightning files...

Cancelling review until then, leaving ui-review, as the UI will not change by that.
Attachment #299148 - Flags: review?(Berend.Cornelius)
FWIW I totally agree with Mickey here, the today pane being hardwired to Lightning is a bug in my opinion. I would favor a full solution, that solution being the ability to display the today pane in all Lightning modes *AND* in Sunbird.

While I agree, that Sunbird currently shows events and tasks in the same UI window, that is not the desired outcome in the longer term. The desired outcome is a separation of the calendar functionalities and task functionalities in Sunbird as it has already happened in Lightning with the calendar mode and task mode.

Comment 28

12 years ago
Sven thanks for the patch. Some comments from an ui-point of view:
  
* Add the Today Pane Button to the Calendar Mode Toolbar [1] and the
  upcoming Task Mode Toolbar (Bug 327780)

* Add the entry Today Pane to the Calendar Mode & Task Mode menu [2]

* Allow users to toggle the visibility of the Today Pane for each mode independently

* Allow users to customize the content (Events & Task, Events, Tasks) within the pane for each mode separately

* With a clean profile the Today Pane should be switched off for all three modes by default.

* Last but not least I second the request that the Today Pane should be available for Sunbird. But before starting with that we should think about the problems we want to solve with that change. Adding the Today Pane to Sunbird without doing some ui-tweaks would duplicate too much information.

In general we should start working on a concept which allows users to customize their views in the flexible way.... But this needs to fixed in another bug.

[1] http://spreadsheets.google.com/pub?key=plCAueWeXt4jIoK2sb3c0QQ&output=html
[2] http://spreadsheets.google.com/pub?key=plCAueWeXt4hSxoRkxWFJCQ

Comment 29

12 years ago
I know a lot of users would like to see this in Lightning. With the above comments this probably won't make the 0.8-train. I know temp-fixes aren't a good thing so maybe someone could create a seperate addon and we could add a relnote to 0.8?

Comment 30

12 years ago
This is the (currently) best solution I can provide, with the following comments:

- There's no toolbar button in calendar/task mode.
This would produce various (later unneeded) lines in CSS files and depends on bug 327780; so I would suggest to file a spin-off bug.
Christian: Would you please the right position for 'Today-Pane' in your spreadsheet [1]?

- 'agenda-listbox.xml/.js' is still located in 'mozilla/lightning/content', although it is related to the 'today-pane.xul' in 'mozilla/calendar/base'.
Could anybody help me with this, because I don't know where to align the Makefiles... (normally Mickey did this for me ;-) ...)

- the button to toggle TodayPane is currently in 'lightning-toolbar.inc', but the style is defined in 'calendar-toolbar.css' (same comment as above)

- there's still one locale for the TodayPane missing (calendar.context.button.accesskey) which is errornous in 'lightning.properties': I used a workaround by setting the accesskey while initializing...
Should we change this after string freeze, or after the release of 0.8???

- the key F11 is defined in Lightning only: is this right or should it be changed as well?


- There are at least 3 steps necessary to include the TodayPane in Sunbird:
  * give one location by adding 'today-splitter' and 'today-pane-panel' (which will be overlaid by 'today-pane.xul')
  * add a toolbar and menu item to toggle TodayPane
  * execute 'TodayPane.prepareCalendarTodayPane( toolbar )' with a toolbar to add the butten automatically
I did not test this, but I think this is also an issue for a spin-off...


According to comment #29, I would suggest to create an extension for the toolbar buttons in calendar/task mode when this is wanted.

Awaiting your responses ;-)
Attachment #299148 - Attachment is obsolete: true
Attachment #299985 - Flags: review?(Berend.Cornelius)
Attachment #299148 - Flags: ui-review?(christian.jansen)
Sven, thanks for the patch. One general comment first:

- I saw some whitespace-related changes. Please provide another diff with the 
  -uw parameters to make it easier for the reviewer to see the code-related 
  changes.

> - There's no toolbar button in calendar/task mode.
> This would produce various (later unneeded) lines in CSS files and depends 
> on bug 327780; so I would suggest to file a spin-off bug.

That one should be pretty easy to fix. Just do

- mode="mail"
+ mode="mail,calendar,task"

on the toolbarbutton.

> - 'agenda-listbox.xml/.js' is still located in 'mozilla/lightning/content',
>   although it is related to the 'today-pane.xul' in 'mozilla/calendar/base'.
>   Could anybody help me with this, because I don't know where to align the
>   Makefiles... (normally Mickey did this for me ;-) ...)

I'd suggest that you just remove the two files from calendar/lightning/content 
and add them to base/content. You'll also need to adjust the jar.mn files in 
calendar/lightning and calendar/base for this.

> - the button to toggle TodayPane is currently in 'lightning-toolbar.inc', 
    but the style is defined in 'calendar-toolbar.css' (same comment as above)

If you need the button in calendar/base/content/calendar-toolbar.inc then 
just move the XUL code to this file.

> - there's still one locale for the TodayPane missing
> (calendar.context.button.accesskey) which is errornous in
> 'lightning.properties': I used a workaround by setting the accesskey while
> initializing...
> Should we change this after string freeze, or after the release of 0.8???

IMO this patch should go in after 0.8 is out either way, but this is just 
my personal opinion.

Comment 32

12 years ago
(In reply to comment #31)
> Sven, thanks for the patch. One general comment first:
> 
> - I saw some whitespace-related changes. Please provide another diff with the 
>   -uw parameters to make it easier for the reviewer to see the code-related 
>   changes.

Done.

> That one should be pretty easy to fix. Just do
> 
> - mode="mail"
> + mode="mail,calendar,task"

Oh I always hoped, it would be that easy! But it's still only displayed in Mail mode. I already thought about such "multi-mode buttons" and I'm happy to see this already implemented for menu, although it's only used for 'calendar,task' - but I would like to wait for bug 327780 on that.

> I'd suggest that you just remove the two files from calendar/lightning/content 
> and add them to base/content. You'll also need to adjust the jar.mn files in 
> calendar/lightning and calendar/base for this.

I will work on another add-on patch, but it should not be related to this review.

> If you need the button in calendar/base/content/calendar-toolbar.inc then 
> just move the XUL code to this file.

Yes, but only after adding TodayPane to Sunbird, so maybe we can live with this for now. It was only a comment.

> IMO this patch should go in after 0.8 is out either way, but this is just 
> my personal opinion.

That would be sad, but this is also just my personal opinion...
Attachment #299991 - Flags: review?(Berend.Cornelius)

Updated

12 years ago
Attachment #299985 - Flags: review?(Berend.Cornelius)

Comment 33

12 years ago
Comment on attachment 299991 [details] [diff] [review]
Same patch with 'diff -uw' for easier review.

Oh sorry:

overlay chrome://messenger/content/messenger-overlay-sidebar.xul chrome://calendar/content/today-pane.xul

should be

overlay chrome://lightning/content/messenger-overlay-sidebar.xul chrome://calendar/content/today-pane.xul

I hope you can correct this without another posting of the patch...

Comment 34

12 years ago
One could use this extension to add the TodayPane toolbar button to calendar-toolbar. (Sorry, no automoatic adding yet...)

Comment 35

12 years ago
(In reply to comment #30)

> - There's no toolbar button in calendar/task mode.
> This would produce various (later unneeded) lines in CSS files and depends on
> bug 327780; so I would suggest to file a spin-off bug.
> Christian: Would you please the right position for 'Today-Pane' in your
> spreadsheet [1]?

I already did, but I didn't press the re-publish button...
It should work now.
see ->http://spreadsheets.google.com/pub?key=plCAueWeXt4jIoK2sb3c0QQ&output=html


> 
> - the key F11 is defined in Lightning only: is this right or should it be
> changed as well?

This should work in all three modes...


Comment 36

12 years ago
(In reply to comment #34)
> Created an attachment (id=300348) [details]
> Extension to show TodayPane toolbar button in Calendar mode
> 
> One could use this extension to add the TodayPane toolbar button to
> calendar-toolbar. (Sorry, no automoatic adding yet...)
> 

This has to be downloaded as attachment.cgi and then renamed to name.xpi (perhaps better provide a zip?). It does add the capability to add the button to the calendar-bar (and task-bar) but for the rest the button doesn't work for me, no error in console...

Comment 37

12 years ago
@Christian: Thanks for the update.
Yes, the F11 key works in all 3 modes - but it wouldn't in Sunbird without redefining there.

@Bas: Yes, please excuse the short explanation...
This is only an add-on for the provided patch. You have to apply this first and then use the extension to have the button in Calendar and Task mode.

Another version with a complete patch and the button in all three modes depends on bug 327780 (Task mode toolbar) to be fixed...

Comment 38

12 years ago
I never knew there is the F11 shortcut. I doubt this works on a lot of Macs (up to last fall with the aluminum keyboards?) where F11 is taken by the system for Expose, so it's not available in Lightning. (applies to all of F9-F12)

Comment 39

12 years ago
Thomas, then the only way to toggle is by toolbar button or menu.
I'd suggest to file another bug to use a key that is available on Mac too.

Comment 40

12 years ago
(In reply to comment #37)
> @Christian: Thanks for the update.
> Yes, the F11 key works in all 3 modes - but it wouldn't in Sunbird without
> redefining there.

Are you sure? On Windows F11 seams to work for both, Lightning and Sunbird, but I haven't verified that on Gnome/KDE.

Regarding the Mac: For me it would be ok to assign no key. The Message Pane (View -> Layout, F8 on Win/Unix) has no key assigned either. 

Comment 41

12 years ago
What do you mean with "works for Sunbird"?
Did you already include the today pane there?

I did not try anything, I just wanted to mention, that I found the following line in 'messenger-overlay-sidebar.xul' (which is Lightning-only):

    <key id="todaypanekey" command="cmd_toggleTodayPane" keycode="VK_F11"/>


Updated

12 years ago
Attachment #299991 - Flags: review?(Berend.Cornelius)

Comment 42

12 years ago
In this file I cut off the moving of the files 'agenda-listbox.js/xml', because I did not know how to diff these right; I do get empty files instead of deleting them. I will post another add-on patch, but I would recommend to move these files from 'mozilla/calendar/lightning/content/' to 'mozilla/calendar/base/content/' manually.

I found the following issues:

1. I appied all UI suggestions from Christian, even the toolbar buttons in calendar/task modes. But because of this, the 'Show Today Pane' toolbar button is available twice when customizing the calendar toolbar, until Berend's patch for bug 327780 is checked-in.
After check-in of that patch, the 'task-show-todaypane-button' button should be added to the defaultset of task-toolbar.

2. While testing, I recognized that Berend's 'addtoolbarbutton' only works for the first session in a clean profile; after restart the button disappears. I checked this to happen without my patch too, so I suggest to file another bug for that.

3. I chose to change locale files to get rid of my former work-around (assign the accesskey for menu-item to toggle the Today Pane). I hope this won't delay the check-in after the release of 0.8, because bug 327780 also needs some further strings and this one only moves them from 'calendar.properties' to 'calendar.dtd'.

4. I also tried functionality in Sunbird, which would work with the already mentioned steps - but there's a problem with duplicated IDs for the task-list so I would wait for the task-mode to be implemented in Sunbird and file a spin-off for this later (as already agreed by others).

I will attach these 2 patches too:
- moving of the files 'agenda-listbox.js/xml'
- this same patch without '-w' to align whitespaces while checking in
Attachment #299985 - Attachment is obsolete: true
Attachment #299991 - Attachment is obsolete: true
Attachment #300348 - Attachment is obsolete: true
Attachment #301672 - Flags: review?(Berend.Cornelius)

Comment 43

12 years ago
Attention: this one does not apply very well; it leaves the files empty in 'mozilla/calendar/lighning/content/'.

Updated

12 years ago
Depends on: 327780
Flags: wanted-calendar0.8?
This should go in after 0.8 is out, especially because of the string change.
Flags: wanted-calendar0.8? → wanted-calendar0.8-
Assignee

Comment 46

11 years ago
This was really a big patch! Naturally it was quite bitrotten meanwhile and I had to invest some time to get it applied again. I tested it and found no errors in the console and the functionality worked - nearly - 100% -> great job!

When I examined the code I noticed the following:
-You put the implementation of the today-menuitems to the XUL file -> I am fine with this. Your introduced resource in calendar.properties is therefor not needed anymore.
-I am not happy that you shifted so much of the implementation from Todaypane.js to messenger-overlay-sidebar.js. I think even when the shifted code is majorly dealing with menus it should remain where it was simply because it belongs to the today-pane and to nowhere else. 
- This is also true for your implementation of 'ltnRestoreLastTodayPaneView' that is called from the methods 'ltnswitch2...' . This whole architecture is not really elegant and should be overworked some day. And for this reason the todaypane listens to selection of the display-deck and reacts accordingly. I really do not want to change this. For the future I have in mind that we fire a global event when the mode switches so that all attached listeners can react on this. But the major point is that we should keep that logic inside the todaypane.

- Similarily I object your move of 'ltnAddButtonToDefaultset(toolbarSetString) and 'ltnAddButtonToToolbarset.' to "messenger-overlay-sidebar.js". Unless you change the signature of these function, so that they can be used for other purposes too, I would like to keep them in todaypane.js, too.

-The "collapsed" attribute of all the modes is handled within the functions 'ltnRestoreLastTodayPaneView()' and 'onModified'. It all works fine and I especially like that you also care about the subpanes and display them mode-dependently. Yet I just want to suggest to find a more universal way to handle the "collapsed" attribute mode-dependently. E.g. we could define a "mode" attribute  at the todaypanebox with the value "mail,calendar,task" and define a second attribute "collapsedinmodes" with the value "true,false,true" or something and a global function that controls this (+ an observer that controls the splitter state). I bet that some day in the future we will need that again in a similar context.

- You moved the today-pane-splitter to messenger-overlay-sidebar.xul. I also would keep this where it is, especially because we only refer to this splitter from the today-pane. In this respect I noticed that you collapse the splitter where you actually should collapse the today-pane as done in my old function "toggleTodaypaneInMailMode". With your implementation it is no longer possible to open the collapsed todaypane by mouse. You should correct this.

- One little UI aspect: I would add a menuseparator before the menuitem "Today pane" from the others in Calendar mode and task mode.

You changed the overlay of the today-pane, so that it now overlays the messenger-overlay-sidebar, whereas before the messenger window got directly overlaid. Maybe it's better so especially because all other panes do it just the same way. On the other hand the today-pane does not have anything to do with the sidebar directly by definition, because it's on the other side of the messenger pane. Maybe I am exaggerating, I just remember we had some weird side effects with these overlays.

Although this patch is really a great improvement I have to deny the review because of the above mentioned issues.

Assignee

Comment 47

11 years ago
Comment on attachment 301672 [details] [diff] [review]
patch v3 ('-w' for review and without moving 'agenda-listbox.js/xml')

denying patch because of the previously mentioned issues.
Attachment #301672 - Flags: review?(Berend.Cornelius) → review-
Assignee

Comment 48

11 years ago
Posted patch unbittrotted patch of v3 (obsolete) — Splinter Review
I unbitrotted the last patch and removed the hunk calendar.properties.

Comment 49

11 years ago
(In reply to comment #46)
> -You put the implementation of the today-menuitems to the XUL file -> I am fine
> with this. Your introduced resource in calendar.properties is therefor not
> needed anymore.

I'm a bit confused now... I realized, that these are not needed and removed them in my patch. You seem to remove my removal, so they stay in there, right?

> -I am not happy that you shifted so much of the implementation from
> Todaypane.js to messenger-overlay-sidebar.js. I think even when the shifted
> code is majorly dealing with menus it should remain where it was simply because
> it belongs to the today-pane and to nowhere else. 

I tried to carefully seperate BASE and LIGHTNING code - I'm not sure, if I was successfully here. But you should not access Lightning elements from todaypane scripts, since it will (probably) appear in Sunbird some time.
If not, we should shift the whole todaypane code to Ligthning.

> - This is also true for your implementation of 'ltnRestoreLastTodayPaneView'
> that is called from the methods 'ltnswitch2...' . This whole architecture is
> not really elegant and should be overworked some day. And for this reason the
> todaypane listens to selection of the display-deck and reacts accordingly. I
> really do not want to change this. For the future I have in mind that we fire a
> global event when the mode switches so that all attached listeners can react on
> this. But the major point is that we should keep that logic inside the
> todaypane.

Same as above.

> - Similarily I object your move of 'ltnAddButtonToDefaultset(toolbarSetString)
> and 'ltnAddButtonToToolbarset.' to "messenger-overlay-sidebar.js". Unless you
> change the signature of these function, so that they can be used for other
> purposes too, I would like to keep them in todaypane.js, too.

Well, of course we could make this more general - but I moved it here, because this is also only needed for Lightning, since Sunbird is able to provide it's own and updated default set with every version.

> -The "collapsed" attribute of all the modes is handled within the functions
> 'ltnRestoreLastTodayPaneView()' and 'onModified'. It all works fine and I
> especially like that you also care about the subpanes and display them
> mode-dependently. Yet I just want to suggest to find a more universal way to
> handle the "collapsed" attribute mode-dependently. E.g. we could define a
> "mode" attribute  at the todaypanebox with the value "mail,calendar,task" and
> define a second attribute "collapsedinmodes" with the value "true,false,true"
> or something and a global function that controls this (+ an observer that
> controls the splitter state). I bet that some day in the future we will need
> that again in a similar context.

Oh, I'm happy with every better suggestion. I also didn't like the introdution of 6 (!) new attributes, but Christian definitely wanted these 2 attribute mode-dependent. Indeed, your proposal would be more smart.

> - You moved the today-pane-splitter to messenger-overlay-sidebar.xul. I also
> would keep this where it is, especially because we only refer to this splitter
> from the today-pane. In this respect I noticed that you collapse the splitter
> where you actually should collapse the today-pane as done in my old function
> "toggleTodaypaneInMailMode". With your implementation it is no longer possible
> to open the collapsed todaypane by mouse. You should correct this.

Same as above, this was my only obvious solution to get rid of Thunderbird IDs in today-pane.xul - I'm fine with every better solution.

> - One little UI aspect: I would add a menuseparator before the menuitem "Today
> pane" from the others in Calendar mode and task mode.

I would agree - I just took Christians proposal from [1].

> You changed the overlay of the today-pane, so that it now overlays the
> messenger-overlay-sidebar, whereas before the messenger window got directly
> overlaid. Maybe it's better so especially because all other panes do it just
> the same way. On the other hand the today-pane does not have anything to do
> with the sidebar directly by definition, because it's on the other side of the
> messenger pane. Maybe I am exaggerating, I just remember we had some weird side
> effects with these overlays.

I just adopted the overlay from others, feel free to keep it - but keep Sunbird in mind...

> Although this patch is really a great improvement I have to deny the review
> because of the above mentioned issues.

We have plenty of time, since wanted-calendar0.8 was denied.
Maybe you find some time to re-think your imlpementation and keep Sunbird in mind. Maybe there is also a F2F meeting in the near future and somebody wants to discuss about that...

[1] http://spreadsheets.google.com/pub?key=plCAueWeXt4hSxoRkxWFJCQ
Assignee

Comment 50

11 years ago
Of course it is a good idea to separate "lightning" code from "Base" code at this stage, but anyway I think that "messenger-overlay-sidebar" is not a good place for today-pane-related lightning code. This should be be within the today-pane itself. Why not make a deduction from Todaypane in "base/content" located in "lightning/content". For the separation of the code I would keep in mind that - hopefully - we will also provide different modes in Sunbird, too  and as soon as this is available it would be good if the todaypane will embed into it with as little code adaption as possible. As far as I can see only the insertion of the menuitem and the code around the mail-toolbar is really absolutely lightning only - the rest could basically also be used by Sunbird with some little adaptions there.
An open issue in in this respect is also how the today-pane and other objects should be notified about modechanges. Currently they listen to a selection change of the lightning-specific control "displaydeck". I suggest to fire an event, e.g. "fireevent("modechange")", that the todaypane could listen to, when the mode changes.

> I'm a bit confused now... I realized, that these are not needed and removed
> them in my patch. You seem to remove my removal, so they stay in there, right?

I am sorry, I was too superficial and removed the hunk. I will add an updated patch.

Looking at the patch again, I find you were right to define the splitter outside the Today-pane in messenger-overlay-sidebar.xul.
Assignee

Comment 51

11 years ago
Posted patch unbittrottedpatch of v3 #2 (obsolete) — Splinter Review
Attachment #306302 - Attachment is obsolete: true

Comment 52

11 years ago
(In reply to comment #51)
> Created an attachment (id=307069) [details]
> unbittrottedpatch of v3 #2

Berend, thanks for your contribution - this patch applies and builds well, although you 'forgot' the modification of the default-sets for calendar ans task mode toolbars. (see this attachment as add-on patch)

Also, the moving of the agenda-listbox files has to be done manually - just remember when trying out these patches.
Attachment #301672 - Attachment is obsolete: true
Attachment #301674 - Attachment is obsolete: true

Comment 53

11 years ago
@Berend, at first an excuse:
I don't have the time to implement your comments in the next weeks. So if you feel able to directly write a patch to fulfill your comments, I would kindly ask you to do so.

(In reply to comment #50)
> Of course it is a good idea to separate "lightning" code from "Base" code at
> this stage, but anyway I think that "messenger-overlay-sidebar" is not a good
> place for today-pane-related lightning code. This should be be within the
> today-pane itself. Why not make a deduction from Todaypane in "base/content"
> located in "lightning/content". For the separation of the code I would keep in
> mind that - hopefully - we will also provide different modes in Sunbird, too 
> and as soon as this is available it would be good if the todaypane will embed
> into it with as little code adaption as possible. As far as I can see only the
> insertion of the menuitem and the code around the mail-toolbar is really
> absolutely lightning only - the rest could basically also be used by Sunbird
> with some little adaptions there.

I would not recommend to produce redundant code.
It would indeed be possible to add a new file for TodayPane support in lightning, but I would just add or maybe modify some routines, without copying too much.
I am also waiting for different modes in Sunbird, or better: for the decision in which direction Sunbird will go... The discussion is still too young to forecast its outcome.

> An open issue in in this respect is also how the today-pane and other objects
> should be notified about modechanges. Currently they listen to a selection
> change of the lightning-specific control "displaydeck". I suggest to fire an
> event, e.g. "fireevent("modechange")", that the todaypane could listen to, when
> the mode changes.

A very good suggestion, but also I would like to wait for Sunbird to support the Task mode in any way.

(In reply to comment #46)
> In this respect I noticed that you collapse the splitter
> where you actually should collapse the today-pane as done in my old function
> "toggleTodaypaneInMailMode". With your implementation it is no longer possible
> to open the collapsed todaypane by mouse. You should correct this.

I decided to do so, to be able to totally hide the today-pane. Without collapsing the splitter, it will always be visible, even if somebody would not like the today-pane in any mode.
I thought it is sufficient to be able to show it by toolbar or menu with the mouse...
But it would be a very small modification...

@Christian: What do you think about that?

Comment 54

11 years ago
The TB Folder Pane shows the splitter if collapsed (Win XP), the Today pane behaves the same way, so we should do for Task- and Calendar Mode. Same things should behave same. ;-)
Assignee

Comment 55

11 years ago
Before going on with this issue I would like to see what comes out of bug 422640 – enable View menu-items. I have the hope that the binding that I introduced there may be used for this issue too in order to display the today-pane and its subpanes mode-dependently. 
Depends on: 422640
Assignee

Comment 56

11 years ago
When we introduce the mode dependent behaviour we should fix Bug 390982 – "Today Pane: splitter not being hidden when pane is closed". on the way and especially consider the comment #9 and comment 10 of that issue.
Assignee

Updated

11 years ago
Blocks: 390982

Comment 57

11 years ago
Okay, I will try to follow Christian's bug 390982 comment 10 when I go on with this...
Reporter

Updated

11 years ago
Summary: Display Today Pane in Calendar mode too → Display Today Pane with Task List in Calendar mode too

Comment 58

11 years ago
Please also consider doing this in a way that will allow bug 387575 to be implemented in the future.  I don't know if the two bugs are directly related to each other, just thought I should mention it.
Flags: wanted-calendar0.8- → wanted-calendar0.9+
Assignee

Comment 59

11 years ago
I adapted the last patches from Sven so that the mode switching is now handled by the modeboxes. I did not fix Bug 390982 –
"Today Pane: splitter not being hidden when pane is closed" on the way unlike proposed by myself in comment #56, because handling the "state" attribute of the splitter mode dependently requires an additional modebox-binding that extends to a splitter, which is not quite as fast to do. I will add a comment in that bug... 
I gave the patch to Andreas to test it and he found there are some other issues to observe: 
-The delete-toolbarbutton in calendar mode is disabled when the focus is  on the today-pane. I'd like to handle this problem (and other similar problems" in a follow-up issue). 
-I don't know if it really makes sense to have a Unifinder-todo-pane in task-mode too. On the other hand it does not disturb, the user can configure it away easily and we provide the same today-pane configuration in each mode. I guess there will be an ongoing discussion about this that I would also like to be handled in a future issue.
Assignee: giermann → Berend.Cornelius
Status: ASSIGNED → NEW
Assignee

Comment 60

11 years ago
Posted patch patch v. #4 (obsolete) — Splinter Review
Of course I would also appreciate Sven's comments on this attached patch as it is based on on his ones.
Attachment #301673 - Attachment is obsolete: true
Attachment #307069 - Attachment is obsolete: true
Attachment #307194 - Attachment is obsolete: true
Attachment #316004 - Flags: ui-review?(christian.jansen)
Attachment #316004 - Flags: review?(philipp)
Assignee

Comment 61

11 years ago
Posted patch patch v. #5 (obsolete) — Splinter Review
patch with a slight improvement. I could see that in the previous patch the toolbarbuttons were not immediatly enabled on focussing the trees in task mode
Attachment #316004 - Attachment is obsolete: true
Attachment #316034 - Flags: ui-review?(christian.jansen)
Attachment #316034 - Flags: review?(philipp)
Attachment #316004 - Flags: ui-review?(christian.jansen)
Attachment #316004 - Flags: review?(philipp)
Comment on attachment 316034 [details] [diff] [review]
patch v. #5

>Index: base/content/agenda-listbox.js
Since the today pane is still lightning only, I think its best kept in lightning/content. The same goes all the other lightning-only features you moved to base/. I can be convinced this is ok, if the today pane is implemented in sunbird before the next release, preferably within the next month.

>+    <command id="calendar_toggle_todaypane_command" oncommand="document.getElementById('today-pane-panel').togglePane(event)"/>
The same goes for commands like this.



>-#ifdef MOZILLA_1_8_BRANCH
>-<box id="mailContent">
>-#else
>-<box id="messengerBox">
>-#endif
What happened to this ifdef? Why don't we need it?

>   do {
>-      node = node.parentNode;
>+      if (node) {
>+          node = node.parentNode;
>+      }

short: node = node && node.parentNode;

>+    for (var i = 0; i < elementcount; i++) {
>+        var element = mailToolbar.childNodes[i];
>+        if (element.localName == "toolbarseparator") {
>+            var separatorindex = toolbarSetString.indexOf("separator");
>+            if (separatorindex > -1) {
>+                var firstSubString = toolbarSetString.substring(0, separatorindex);
>+                var secondSubString = toolbarSetString.substring(separatorindex);
>+                toolbarSetString = firstSubString + ltnTodaypaneButton + "," + secondSubString;
>+            }
>+            buttonisWithinSet = true;
>+            break;
>+        }
Can be done more smoothly with arrays:

     var a = toolbarSetString.split(",");
     var b = a.indexOf("separator");
     if (b > -1) {
       a.splice(b, 0, ltnTodaypaneButton);
       toolbarSetString = a.join(",");
     }

Also, should buttonisWithinSet not be placed inside the if block?


>-}
>+    document.getElementById("modeBroadcaster").setAttribute("mode", gCurrentMode);
>+    ltnInitTodayPane();
>+}    
I'd prefer to make the today pane self-contained, i.e call initialization in a separate load handler somewhere in the today pane code. The same goes for the mode broadcaster, if possible.



r=philipp with comments fixed, especially the location of files.
Attachment #316034 - Flags: review?(philipp) → review+

Comment 63

11 years ago
(In reply to comment #60)
> Of course I would also appreciate Sven's comments on this attached patch as it
> is based on on his ones.

Thanks for doing my work ;-)
Well done, works fine for me - with some tiny nits I will mention later...
(To tell the truth, they already existed in my version)


(In reply to comment #62)
> Since the today pane is still lightning only, I think its best kept in
> lightning/content. The same goes all the other lightning-only features you
> moved to base/. I can be convinced this is ok, if the today pane is implemented
> in sunbird before the next release, preferably within the next month.
I would definitely vote for this, but I fear there are some outstanding decisions regarding the Task Mode in Sunbird... For now we would have duplicated IDs for the Task lists - or we would have to add just the Agenda-Pane in Sunbird.

But apart from that, I thought it is accepted to include the Today-Pane "in future", so why should we keep these files in lightning/content in the meantime? A clean solution would also include to move the existing 'today-pane.js/xul' to lightning/content and then back when implementing the Today Pane for Sunbird?!

> >-#ifdef MOZILLA_1_8_BRANCH
> >-<box id="mailContent">
> >-#else
> >-<box id="messengerBox">
> >-#endif
> What happened to this ifdef? Why don't we need it?
Have a look at the change to overlay 'messenger-overlay-sidebar.xul' instead of 'messenger.xul':

>+% overlay chrome://lightning/content/messenger-overlay-sidebar.xul chrome://calendar/content/today-pane.xul
>-% overlay chrome://messenger/content/messenger.xul chrome://calendar/content/today-pane.xul

> >+    for (var i = 0; i < elementcount; i++) {
> >+        var element = mailToolbar.childNodes[i];
> >+        if (element.localName == "toolbarseparator") {
> >+            var separatorindex = toolbarSetString.indexOf("separator");
> >+            if (separatorindex > -1) {
> >+                var firstSubString = toolbarSetString.substring(0, separatorindex);
> >+                var secondSubString = toolbarSetString.substring(separatorindex);
> >+                toolbarSetString = firstSubString + ltnTodaypaneButton + "," + secondSubString;
> >+            }
> >+            buttonisWithinSet = true;
> >+            break;
> >+        }
> Can be done more smoothly with arrays:
> 
>      var a = toolbarSetString.split(",");
>      var b = a.indexOf("separator");
>      if (b > -1) {
>        a.splice(b, 0, ltnTodaypaneButton);
>        toolbarSetString = a.join(",");
>      }
> 
> Also, should buttonisWithinSet not be placed inside the if block?
@Berend: I think Philipp is right. Is it possible, that the whole loop in mailToolbar.childNodes[] is one of these evel Copy&Paste errors?


Now my observations:
- the added toolbar button (mail-toolbar) is not persistent, when no customization is being done in the first session
(this was already in your first implementation and could be handled in another bug)

- one more thing for the enabling/disabling of toolbar buttons:
  * the 'Delete' button in Calendar mode always acts for selection in the current Calendar view, even if the Today Pane (Agenda or Task list) is focussed
  * the 'Delete' button in Task mode is being disabled when focussing Agenda pane

But again & apart from these issues, thanks for your work!
Assignee

Comment 64

11 years ago
>But apart from that, I thought it is accepted to include the Today-Pane "in
>future", so why should we keep these files in lightning/content in the
>meantime? A clean solution would also include to move the existing
>'today-pane.js/xul' to lightning/content and then back when implementing the
>Today Pane for Sunbird?!

This is also my opinion. Sven has invested some time to separate the lightning specific code from the common code and I don't see any reason why we should turn this back. But I also agree that we could shift back the new base/jar.mn entries to lighning/jar.mn until the agenda-pane is used in Sunbird.

>- the added toolbar button (mail-toolbar) is not persistent, when no
>customization is being done in the first session
>(this was already in your first implementation and could be handled in another
>bug)
For this problem bug 392855 – [Today Pane] 'Show Today Pane' button not available with fresh profile has been filed. I is currently assigned to me but maybe Sven, you would like to take it, too....

>- one more thing for the enabling/disabling of toolbar buttons:
>  * the 'Delete' button in Calendar mode always acts for selection in the
>current Calendar view, even if the Today Pane (Agenda or Task list) is focussed
>  * the 'Delete' button in Task mode is being disabled when focussing Agenda
>pane
I noticed this, too. If it's Ok I would like to deal with this in a follow-up issue. I had a look at the code where the commands are dealt with and it seems they don't really observe which pane has the focus. This deserves some dedicated attention.

Updated

11 years ago
Blocks: 392855

Comment 65

11 years ago
Posted patch patch#6Splinter Review
This is a slightly modified version of 'lightning-today-pane.js', which just changes mailtoolbar.CurrentSet and on the way addresses bug 392855 and Philipp's comment #62.

@Philipp:
I hope you agree, that the two files for Agenda Pane do not hurt inside calendar.jar, since the Today Pane has been in there for several month - and: we all will promise to continue work on integrating the Today Pane into Sunbird...

> >-}
> >+    document.getElementById("modeBroadcaster").setAttribute("mode", gCurrentMode);
> >+    ltnInitTodayPane();
> >+}    
> I'd prefer to make the today pane self-contained, i.e call initialization in a
> separate load handler somewhere in the today pane code. The same goes for the
> mode broadcaster, if possible.
I think this depends on the way of integration for Sunbird...
The initialization in ltnInitTodayPane() is only Lightning specific and there are currently no 'modes' in Sunbird, so the only way would be add another onLoad listener in 'lightning-today-pane.js' - did you intend this?

@Berend: I agreed to handle all remaining issues in follow-ups.
Attachment #316034 - Attachment is obsolete: true
Attachment #316218 - Flags: ui-review?(christian.jansen)
Attachment #316218 - Flags: review?(philipp)
Attachment #316034 - Flags: ui-review?(christian.jansen)

Comment 66

11 years ago
I think we forgot Christian's comment #28:
> * With a clean profile the Today Pane should be switched off for all three
> modes by default.

Could this be easily achived with the current implementation of 'collapsedinmodes'?
It does probably not make sense to insert 'collapsedinmodes="mail,calendar,task"' into the general 'today-pane.xul', since not all modes are required for Sunbird.

@Christian: Could you agree to SHOW the Today Pane by default?

Comment 67

11 years ago
Yes, this is OK. OFF would be nicer of course, because it ruduces complexity.
Anyhow, I'll r+ the bug, but I'd like to see the following things changed in spin-off bugs.

General:
Store Today Pane width Mode dependent

Calendar Mode:
- Show Tasks only by default.
- Hide the Mini Day

Task Mode:
- Show Events only by default.
- Hide the Mini Day

Comment 68

11 years ago
Comment on attachment 316218 [details] [diff] [review]
patch#6

See Comments #67
Attachment #316218 - Flags: ui-review?(christian.jansen) → ui-review+
Assignee

Comment 69

11 years ago
Comment on attachment 316218 [details] [diff] [review]
patch#6

Taking over review as discussed with philipp on irc.
Attachment #316218 - Flags: review?(philipp) → review?(Berend.Cornelius)
Assignee

Comment 70

11 years ago
I tested the patch #6 and started a new profile and after installing lightning I the mail-toolbar was completely empty - except for the today-pane button. Resetting the toolbar to the defaultset restored the mailtoolbar including the today-pane-button. As I investigated the "currentSet" attribute was empty ("") in this case. I inserted a fallback to take the defaultSet then and it worked fine. But anyway I don't feel comfortable to integrate this within this issue. This is a sensitive area and before we change something here I would like to have this tested thoroughly. So I suggest we take the hunk "lightning-today-pane.js of Sven's patch with with my little modifications and add it to bug 392855 to verify it distinctively.
For this patch I want to take patch #5 and check it in.
Talked to philipp and daniel about where we place the agenda files (agenda.xml and agenda.js) and we agreed to move it to base/content expecting the agenda-pane to be integrated to lightning within the following weeks. Sven do you have ambitions to do this?
Sven's patch also addressed some objections by philipp (see comment #62), that we should then also handle within bug 392855.
Assignee

Comment 71

11 years ago
Comment on attachment 316218 [details] [diff] [review]
patch#6

denying patch due to last comment
Attachment #316218 - Flags: review?(Berend.Cornelius) → review-
Assignee

Comment 72

11 years ago
>Talked to philipp and daniel about where we place the agenda files (agenda.xml
>and agenda.js) and we agreed to move it to base/content expecting the
>agenda-pane to be integrated to lightning within the following weeks.

Sorry I meant to say 
"expecting the
>... to be integrated to 'Sunbird' within the following weeks.

Comment 73

11 years ago
I agree to handle the change to lightning-today-pane.js in bug 392855.
I just thought this change has no side-effects and I did not want to wait too long ;-)
But as I realize, that this one is to be closed soon, we may go on testing the persistance in bug 392855.

(In reply to comment #70)
> Sven's patch also addressed some objections by philipp (see comment #62), that
> we should then also handle within bug 392855.

Are you going to integrate Philipp's suggestion right here?

> short: node = node && node.parentNode;

This has nothing to do with bug 392855 and is the only change in my patch outside of lightning-today-pane.js!

(In reply to comment #72)
> Talked to philipp and daniel about where we place the agenda files (agenda.xml
> and agenda.js) and we agreed to move it to base/content expecting the
> agenda-pane to be integrated to 'Sunbird' within the following weeks. Sven do
> you have ambitions to do this?

I already tried that some month ago and it would be quite easy, if there were no other task list...
It would depend on UI decisions where to place the task list or how to integrate the task mode (bug 405508) - I could easily remove the task list from its current location and post a screenshot for UI review -- should we file a spin-off?
Assignee

Updated

11 years ago
Attachment #316034 - Flags: ui-review?(christian.jansen)

Comment 74

11 years ago
Comment on attachment 316034 [details] [diff] [review]
patch v. #5

r+ Changes of patch #6 will be handled in different bug
Attachment #316034 - Flags: ui-review?(christian.jansen) → ui-review+
Assignee

Comment 75

11 years ago
patch checked in on trunk and MOZILLA_1_8_BRANCH

-> fixed

I will file a follow-up bug for the remaining problems.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Assignee

Comment 76

11 years ago
Filed as follow- up issues:

-Bug 429685 – Toggling of delete - toolbarbutton in calendar-mode and task-mode does not work
-Bug 429687 – Follow-up features for the mode dependent today-pane.

Added a patch based on the last denied patch (patch#6) to bug 392855.

Sven, thank you again for your contribution!


Target Milestone: --- → 0.9

Comment 77

11 years ago
Hi,

First of all thanks for providing this feature. Just tried out this feature with the nightly build of the 20th. When I collapse the pane (by dragging the edge of the today pane) and then switch to mail mode and then switch back to the calendar mode the today pane expands. It does not remember it's last expanded/collapsed state. However, it works as expected when I use the F11 shortcut key to collapse and expand the pane. Please take a look into this. Thanks in advance.

Comment 78

11 years ago
Actually it happens for panes in both the views (mail as well as calendar). Thanks.

Updated

11 years ago
Blocks: 430090

Comment 79

11 years ago
Since there are many bugs waiting for this one to close and the described problem is more cosmetical, I was filing a spin-off bug 430090 and taking this one.

It seems we just need to observe the 'collapsed' attribute, but I expect more issues on solving this...
Reporter

Comment 80

11 years ago
This bug probably regressed Bug 431811.
Reporter

Updated

11 years ago
Depends on: 431829

Comment 81

11 years ago
Checked in lightning build 2008062218 -> VERIFIED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.