Closed
Bug 350323
Opened 18 years ago
Closed 17 years ago
show hidden calendars when they are selected.
Categories
(Calendar :: General, defect)
Calendar
General
Tracking
(Not tracked)
RESOLVED
FIXED
0.7
People
(Reporter: mvl, Assigned: thomas.benisch)
Details
Attachments
(2 files, 2 obsolete files)
15.73 KB,
image/png
|
Details | |
3.39 KB,
patch
|
dbo
:
first-review+
|
Details | Diff | Splinter Review |
I personally never liked the way that the selected calendar in the listbox is the calendar in which new items are created. There is no UI hint to that anywhere. You just have to know it. Maybe we need some checkbox in the listbox? One that gets remembered across sessions.
Comment 1•18 years ago
|
||
I tend to agree that it would be nice if it were a bit more discoverable. FWIW, iCal behaves just like we currently do.
Comment 2•18 years ago
|
||
I think the current situation is rather easy to use. If you create new event/task using the dialog there is a drop-down menu showing the calendar the item is created in. Or just select calendar in Calendars tab and create new events with drag'n'drop. You're done. What might be useful is to define a default calendar that is selected right after application start. Maybe remembering the calendar that was selected on shutdown would be enough. Another possibility would be to show the currently selected calendar in the up-to-now unused status bar.
Reporter | ||
Comment 3•18 years ago
|
||
No, the current UI is not easy to use. It's hard to discover the relation between the selected item and the default calendar. And once you discovered it, it is still hard to use. I forget to select the right calendar all the time. We still need a better solution
Comment 4•18 years ago
|
||
(In reply to comment #1) > I tend to agree that it would be nice if it were a bit more discoverable. > FWIW, iCal behaves just like we currently do. > The subtle difference to iCal (and this is our biggest usability issue here) is the fact that you can select a calendar without having its (visibility-)checkmark set. As a result you can enter events into an invisible calendar, which is very confusing. The simple solution to this is: selection of a calendar in the list must automatically toggle the checkbox to be checked. That's exactly how iCal behaves.
Comment 5•18 years ago
|
||
Nice catch, Stefan. That suggestion sounds reasonable to me and has a good intuitive feel about it. Offhand, at least, I can't imagine a situation where it would be necessary to permit invisible calendars to be selected.
Comment 6•18 years ago
|
||
(In reply to comment #4) > the fact that you can select a calendar without having its > (visibility-)checkmark set. As a result you can enter events into an invisible > calendar, which is very confusing. See also related Bug 345410
Reporter | ||
Comment 7•18 years ago
|
||
While adding items to invisible calendar is ineed a problem, it still does not fix my original problem: There is no hint that the selected calendar all the way to the left of the screen has anything to do with where new items end up.
Assignee | ||
Comment 8•18 years ago
|
||
When a calendar in the calendar tab is selected, this patch adds the selected calendar to the composite calendar. The corresponding checkbox is automatically toggled to be checked.
Attachment #248236 -
Flags: first-review?(lilmatt)
Assignee | ||
Comment 9•18 years ago
|
||
I also provided a patch, which selects the stored default calendar in the calendar tab list right after application start (see Bug #363432).
Assignee | ||
Comment 10•18 years ago
|
||
(In reply to comment #9) > I also provided a patch, which selects the stored default calendar in the > calendar tab list right after application start (see Bug #363432). The bug I refered to is duplicate, please see Bug #362930.
Comment 11•18 years ago
|
||
Comment on attachment 248236 [details] [diff] [review] add selected calendar to composite calendar patch This looks good. Could you also port this fix to Sunbird?
Reporter | ||
Comment 12•18 years ago
|
||
As I said before: this patch does not fix the bug as i filed it.
Assignee | ||
Updated•18 years ago
|
Attachment #248236 -
Flags: first-review?(lilmatt)
Assignee | ||
Comment 13•18 years ago
|
||
This patch also includes the port to Sunbird.
Attachment #248236 -
Attachment is obsolete: true
Attachment #248498 -
Flags: first-review?(lilmatt)
Assignee | ||
Comment 14•18 years ago
|
||
(In reply to comment #12) > As I said before: this patch does not fix the bug as i filed it. You're absolutely right, that this patch doesn't fix your described problem. Nevertheless, I think this patch makes sense and improves the usability. So if you don't object to this patch in general, after review this patch can be checked in as part of this bug or alternatively I can file a new bug.
Reporter | ||
Comment 15•18 years ago
|
||
It's was just a reminder: don't go auto-pilot and mark this bug as fixed after checking in. It should be left open. No need for another bug now.
Comment 16•18 years ago
|
||
Comment on attachment 248498 [details] [diff] [review] add selected calendar to composite calendar patch v2 r=lilmatt
Attachment #248498 -
Flags: first-review?(lilmatt) → first-review+
Assignee | ||
Comment 17•18 years ago
|
||
add selected calendar to composite calendar patch checked in on HEAD and MOZILLA_1_8_BRANCH
Reporter | ||
Comment 18•18 years ago
|
||
The checked-in patch does not work for me on sunbird. I still can select an hidden calendar, and can still add items to it. And besides, why was this checked-in without ui-review and second-review?
Comment 19•18 years ago
|
||
Using Sunbird/0.4a1 (2006121504) I now see the following misbehavior caused by the checkin: A calendar in disabled and not selected in calendar list. Now click the checkbox to enable calendar -> calendar stays disabled. I think the reason is that the calendar gets selected first (calendar will be enabled) and than the checkbox gets clicked (calendar will be disabled again).
Comment 20•18 years ago
|
||
Nominating for backout based on comment #18 and comment #19.
Comment 21•18 years ago
|
||
(In reply to comment #20) Agreed. This shouldn't have landed as it doesn't have r2, and it's apparently causing regressions. Please back this out for now.
Assignee | ||
Comment 22•18 years ago
|
||
As requested I removed this patch from CVS. By the way, in which cases is a 2nd review necessary?
Assignee | ||
Comment 23•18 years ago
|
||
Comment on attachment 248498 [details] [diff] [review] add selected calendar to composite calendar patch v2 Before working on an improved version of this patch I'd like to ask for UI review. When a calendar in the calendar tab is selected, the current patch adds the selected calendar to the composite calendar. The corresponding checkbox is automatically toggled to be checked. The questions is, if this behaviour does make sense. Please see also comment #5. In addition I stumbled about the following problem. E.g. in Lightning it is possible, that after a calendar was selected and added to the composite calendar, that the user clicks the checkbox, so that the calendar is removed from the composite calendar again. That means, we have again the situation, that a calendar is selected, which is not part of the composite calendar.
Attachment #248498 -
Flags: ui-review?(mvl)
Comment 24•18 years ago
|
||
How about something like this? Frankly, you guys looking/working on that patch seem to have missed the point. It's a user friendliness issue. Like Michiel said, it's just not obvious that the highlighted calendar is the default. Forcing a hidden calendar to be displayed when adding tasks/events is besides the point. For one thing, the user's view might not be in the range where you create the event (you can change the dates after all). So if you're going to force the visibility of a calendar you'd also logically need to force the current view to change to the start date of the event, otherwise you're not any further ahead than you were. I for one, probably wouldn't like that feature. I have it working as depicted for Lightning, I have the xpi, no cvs access right now, not sure what's involved in that, haven't looked though. If anyone wants the xpi I can attach.
Reporter | ||
Comment 25•18 years ago
|
||
Changing the summary to indicate that this bug has been morphing (sad as it is). bug 366914 filed for the original issue.
Summary: make default calendar for new items more obvious → show hidden calendars when they are selected.
Comment 26•17 years ago
|
||
Just to voice my opinion on this bug for the new summary. I don't think this should be implemented. For one thing, if I accidentally click a non-visible calendar which has many items, it takes time to load (ok a few seconds, but still). I prefer the visibility checkbox. Maybe I'm unique in this, but for another example, I have one calendar I keep invisible which I use to house a few recurring events. I don't need to see these events on my calendar (they popup notifications), but I might want to add one without having to wait a second or two for all existing items to display. I only need to see them if I want to edit something.
Reporter | ||
Comment 27•17 years ago
|
||
I'm not sold on the proposed patch. What is the problem it tries to solve? Adding items to hidden calendars?
Assignee | ||
Comment 28•17 years ago
|
||
(In reply to comment #27) > I'm not sold on the proposed patch. What is the problem it tries to solve? > Adding items to hidden calendars? I'll describe the problem this patch tries to solve in more detail. You select a calendar in the calendar tab where the checkbox is not checked, that means the calendar is not part of the composite calendar. Then you create an event, either by dragging in the view or by pressing the new event button. The event dialog uses the selected calendar, that means the event is created in the hidden calendar. After you've finished with creating the event, the event is not diplayed in the view. When creating the event by dragging, it seems as if your event creation failed. By adding a selected calendar to the composite calendar this problem is solved.
Reporter | ||
Comment 29•17 years ago
|
||
Comment on attachment 248498 [details] [diff] [review] add selected calendar to composite calendar patch v2 Hmm, i guess this does solve some problem. ui-review=mvl I prefer the early-return if there is no calendar chosen (like it originally was in calendarManagement.js). But i'll leave that up to you. r2=mvl
Attachment #248498 -
Flags: ui-review?(mvl)
Attachment #248498 -
Flags: ui-review+
Attachment #248498 -
Flags: second-review+
Comment 30•17 years ago
|
||
Based on comment #28, I would suggest that the only problem this solves is when you create the event by dragging. As I stated in comment #24, there's no guarantee that an event created using the new event button will even be in the range of the current view, so this proposed patch doesn't solve the supposed issue of the event not being in the view after it's created. However, the solution I would envision for dragging, instead of adding the hidden calendar to the composite: 1. have the event slowly fade away and/or 2. immediately open an edit event dialog for the event. The problem as I see it is that because it disappears right away, there's currently no way other than adding the hidden calendar to the composite to edit that event.
Assignee | ||
Comment 31•17 years ago
|
||
This patch uses the early return proposed in comment #29. In addition this patch fixes the problem from comment #19. tbe->mvl: I'm not sure, if the problem from comment #18 is fixed. If not, can you please elaborate your problem in more detail. Due to the changes in the patch I ask for r1 again. Is it necessary to have r2 and ui-review again?
Assignee: nobody → thomas.benisch
Attachment #248498 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #256150 -
Flags: first-review?(mvl)
Reporter | ||
Comment 32•17 years ago
|
||
Comment on attachment 256150 [details] [diff] [review] add selected calendar to composite calendar patch v3 >+ clearTimeout(gCalendarListSelectTimeout); This timeout looks like a hack to me. It really needs comments about why it's needed etc. But i very much prefer not needing it at all. How about calling event.preventDefault in the onCalendarListSelect(), when you detected that the calendar needs to be added? That way, the checkbox won't receive the call. I think. If event bubbles the other way (i can't remember right now), you can stop the bubbling somewhere else. Either way, make sure the other handler won't get called.
Assignee | ||
Comment 33•17 years ago
|
||
(In reply to comment #32) > (From update of attachment 256150 [details] [diff] [review]) > >+ clearTimeout(gCalendarListSelectTimeout); > This timeout looks like a hack to me. It really needs comments about why it's > needed etc. I agree that the timer approach is quite hacky. But I haven't found a better way to fix this task. > But i very much prefer not needing it at all. How about calling > event.preventDefault in the onCalendarListSelect(), when you detected that the > calendar needs to be added? That way, the checkbox won't receive the call. I > think. If event bubbles the other way (i can't remember right now), you can > stop the bubbling somewhere else. Either way, make sure the other handler won't > get called. This won't work. I'll try to describe the problem in more detail. The idea is to add the selected calendar to the composite calendar in all cases, except if the user has clicked with the mouse on the checkbox. This exception is necessary, because otherwise it's not possible for the user to add or remove calendars to or from the composite calendar. A list item can be selected by the user in two different ways: a) clicking on the list item ------------------------- In this case first a select event is fired (onCalendarListSelect() handler), after that a click event is fired (onCalendarCheckboxClick() handler). Only in the click handler it is known, if the user clicked on a checkbox. b) selecting the list item by using the keyboard --------------------------------------------- In this case only a select event is fired. I tried to handle the above described exception in that way, that the select handler sets up a timer, which schedules adding the selected calendar to the composite calendar. If the user clicked on a checkbox, the timer is cancelled in the click handler. If I add the calendar to the composite calendar already in the select handler, then the click handler removes the calendar again in the click handler, if the user clicked on a checkbox. This problem is described in comment #19. If I handle adding the calendar to the composite calendar completely in the click handler, I have a problem with b). An alternative solution I had in mind was to have the same behaviour as in the Lightning calendar tab, which uses a tree list box. The list item is only selected, if the user clicks on the non-checkbox area of the item. If the user clicks on the checkbox, the checkbox is toggled, but the item is not selected. This means, that a select event is only fired, if the user clicks on the non-checkbox area. But I didn't find a way to get this running, because this means changing the default behaviour of a list box. Of course one can think about using a tree listbox instead of a listbox ...
Assignee | ||
Updated•17 years ago
|
Attachment #256150 -
Flags: first-review?(mvl) → first-review?(daniel.boelzle)
Comment 34•17 years ago
|
||
Comment on attachment 256150 [details] [diff] [review] add selected calendar to composite calendar patch v3 Using a timer is actually a hack, of course. IMO the code needs verbose documentation about the hack, marked XXX, because there is room for improvement. r=dbo
Attachment #256150 -
Flags: first-review?(daniel.boelzle) → first-review+
Updated•17 years ago
|
Whiteboard: [checkin needed after 0.5]
Assignee | ||
Comment 35•17 years ago
|
||
patch checked in on trunk and MOZILLA_1_8_BRANCH
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Updated•17 years ago
|
Whiteboard: [checkin needed after 0.5]
Target Milestone: --- → 0.7
You need to log in
before you can comment on or make changes to this bug.
Description
•