639447 - Improve Selection UI

Status: ASSIGNED

(Calendar :: Calendar Frontend, defect)

Description

[Philipp Kewisch [:Fallen][📱📆]]

Currently, when selecting a calendar item and then moving focus it is not always clear if the item is in the current selection. For example, STR:


1. Go to mail mode
2. Select an item from the today pane
3. Select a folder item from the folder pane


This causes the calendar item to be shown in bright blue, even though the folder pane is currently in focus.

In this bug I will be creating a manager interface so we can handle selection and focus better.

Comment 1

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v1

This patch adds a selection manager that takes care of the currently focused selection. The different parts of the calendar UI now notify this manager when they receive or loose focus.

This has made the specific commands for modify/deleting the focused items obsolete and also its no longer needed to differ between tasks and events, its merely items w.r.t selection.

I was also able to make the circular dependancy between the views and unifinder a bit more intuitive since the selection manager allows listening for selection changes.

I've also changed the CSS a bit so that its more visible to the user which pane is currently active. Similar to the already existing behavior in the tree views, when the today pane is not in focus then instead of the selected blue color, a gray inactive color is shown. 

This also lead to finding a bug with showing the currently happening event: If an event is currenly occurring, then it is shown in light blue. If you previously selected this event, then selected something else, the light blue went away. I've fixed this by changing the attribute name from "current" to "currentEvent".

The Sunbird changes are untested and don't have to be reviewed thourougly (NPOTB).

Since this patch is large, if you need support for reviewing please let me know and I'll set additional review to someone else.

Comment 2

[Philipp Kewisch [:Fallen][📱📆]]

Attachment: Fix - v2

In addition to the previous comments, this patch removes a few unneeded functions and simplifies getting items of a specific type:

* getSelectedTasks() gets replaced with gCalendarSelection.getSelectedItemsType()
  which gets items that implement a specific interface

* getTaskTree() is removed since there was only one caller left, replaced with
  document.tooltipNode which works fine.

Comment 3

[Decathlon]

Attachment: Selected items on Windows 7

Just tested the fix-v2 on Thunderbird 3.3a4 (Miramar) after the building troubles. :-)

(In reply to comment #1)
> I've also changed the CSS a bit so that its more visible to the user which pane
> is currently active. Similar to the already existing behavior in the tree
> views, when the today pane is not in focus then instead of the selected blue
> color, a gray inactive color is shown. 

This doesn't seem working.
When I click on an event on the today pane, I can't get the normal look of a selected event i.e. the bright(selected) blue color. I can get the bright blue only if I press the tab key (as many time as needed to move the focus on the agenda). A normal selection by clicking on the event, gives to the events the gray inactive color, but then, clicking another element (a task in the task section, a mail or a folder in the mail pane), the event doesn't change look, it stays gray so it's impossible to understand if the event is selected or focused/blurred.


> This also lead to finding a bug with showing the currently happening event: If
> an event is currenly occurring, then it is shown in light blue. If you
> previously selected this event, then selected something else, the light blue
> went away. I've fixed this by changing the attribute name from "current" to
> "currentEvent".

This now seems working fine.


I've also found these wrong behaviors:

1)
The events on the todaypane can't be neither opened nor deleted by double click or by contextual menu items. The only possible action is converting to mail or task (the only item active in the contextual menu). Once converted into a task (save and close the dialog but *without* click any element in the view) then the items "Open" and "Delete event" in the contextual menu becomes active, though, opening or deleting any event actually acts on the just converted task and not on the selected event.
Once done the conversion into a task by saving and closing the dialog, if you click in the view (select an event or change the selected day), in the contextual menu, the items "Open" and "Delete event" become inactive as before.

2) 
I don't know how this could depend on the patch but I verify this behavior in the task view:
- create a new task;
- go in task mode and select the task;
- double click the task in order to open the edit dialog (the dialog gets
  the focus)
  -> the buttons on the task message-header disappear;
- close the edit dialog
  -> the buttons on the task message-header reappear;


Another thing related to the Windows 7 (aero?) theme (that actually is matter for another bug):
all the items in the system, selected or blurred, have a look that is that one you can find e.g. on the Thunderbird's folder pane as you can see in the screenshot (I don't know whether this is customizable somehow via Windows' settings) i.e. a transparent blue color with a thin border (darker when selected, clearer when blurred/hovered). The same look is present in the todaypane's tasks section and in the unifinder. Instead, the look on the agenda is different (apart from the issue) and maybe would be better a more similar look.

Now I'm going to take a look to the code.

Comment 4

[Decathlon]

Forgot to say that the contextual menu in the task section of today pane is broken too. Only the item "New task..." is active, but here, deleting the tasks with the "del" key or opening them with double click is working.

Comment 5

[Decathlon]

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

Found another issue related to bug 632174

Start Thunderbird with unifinder already closed in the previous session, switch to calendar mode, drag and drop (or open) any event in the view. The console shows the error:

Error: TypeError: this.tree.view is undefined STACK: 1: [resource://calendar/modules/calUtils.jsm -> file:///C:/.../calendar-js/calUtils.js:1236] notifyFunc
2: [resource://calendar/modules/calUtils.jsm -> file:///C:/.../calendar-js/calUtils.js:1239] calListenerBag_notify
3: [resource://calendar/modules/calSelectionManager.jsm:214] notifySelectionChanged
4: [chrome://calendar/content/calendar-month-view.xml:665] setSelectedItems
5: [chrome://calendar/content/calendar-view-core.xml:298] select
6: [chrome://calendar/content/calendar-view-core.xml:382] onxbldraggesture

Source File: resource://calendar/modules/calUtils.jsm -> file:///C:/.../calendar-js/calUtils.js
Line: 1236

The error occurs only the first time an event is moved.

The error is different from that one reported in bug 632174 but the cause is probably the same, though, that error occurs, instead of the one mentioned above, if you start TB with unifinder open and then you close it before move the event. Starting TB with unifinder closed, and moving an event, generates the error reported here that is specific of the code present in the patch.

All considered, I think would be better to take a look to another patch with fewer issues.
r-

Comment 6

[Philipp Kewisch [:Fallen][📱📆]]

Decathlon, thank you very much for your elaborate review. I had hoped my patch would "just work", but good thing you found the errors before checkin.

Given Thunderbird 5 is due around the 21st, I'm going to postpone this until after the release.

Comment 7

[Markus Adrario [:Taraman]]

Whats the status on this one?

Since selections seems to be quite buggy, this one should get some attention.

Comment 8

[Philipp Kewisch [:Fallen][📱📆]]

I haven't done anything since the patch was uploaded. As mentioned in the other bug, feel free to take over :-)