368558 - add colors to Lightning calendars tab

Status: VERIFIED FIXED

(Calendar :: Lightning Only, defect)

Description

[Thomas Benisch]

The calendar colors are not displayed in the Lightning calendars tab.
This is a major usability issue.

Comment 1

[Thomas Benisch]

Attachment: calendar colors

This patch implements color boxes for Lightning's calendars tab.
The color boxes are added between the checkbox and the calendar name,
see http://wiki.mozilla.org/Calendar:Calendar_View for more details.

Comment 2

[Thomas Benisch]

Attachment: calendars tab screenshot

This is a screenshot of the calendars tab with color boxes added.

Comment 3

[Martin Schröder [:martinschroeder]]

See bug 298360 for the same issue.

Comment 4

[Thomas Benisch]

(In reply to comment #3)
> See bug 298360 for the same issue.

Thanks, I didn't find this one.

Should I set this issue to duplicate and add my patches to Bug #298360?

Comment 5

[Matthew (lilmatt) Willis]

I'd dupe it the other way around.  This bug has more recent patches (read: less bit-rot)

Comment 6

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 253168 [details] [diff] [review]
calendar colors

ui-review=mvl

Comment 7

[Matthew (lilmatt) Willis]

Comment on attachment 253168 [details] [diff] [review]
calendar colors

>-        if (col.id == "col-calendar-Checkbox") {
>+        if (col.id == "col-calendar-Checkbox" || col.id == "col-calendar-Color") {
Please move col.id == "col-calendar-Color" to the next line.

>+function getDefaultCalendarColor() {
>+    return "#A8C2E1";
>+}
Rather than hardcoding this, it should really be in a custom CSS selector of its own in lightning.css, so that it can be changed by theme authors, etc.

r=lilmatt with those.

Comment 8

[Thomas Benisch]

Attachment: calendar colors v2

This patch fixes all remarks from comment #7.
I carried over the ui-review+ and first-review+ flags
from the last patch.

Comment 10

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 253314 [details] [diff] [review]
calendar colors v2

> +    return "color-" + (color ? color.substr(1) : "default");

Add a comment explaining that you strip the #

r2=mvl with that

Comment 11

[Thomas Benisch]

Thanks for those fast reviews.

Patch checked in on HEAD and MOZILLA_1_8_BRANCH.

Comment 12

[Thomas Benisch]

I reopened this bug because I found another problem.
After application startup the color boxes for unchecked
calendars (calendars which are not in the composite calendar)
are not shown. Only after selecting those calendars the
color boxes are shown.

Comment 13

[Thomas Benisch]

Attachment: missing color boxes for unchecked calendars

This patch fixes the problem with the missing color boxes
for unchecked calendars after application startup.

Comment 14

[Michiel van Leeuwen (email: mvl+moz@)]

Comment on attachment 258541 [details] [diff] [review]
missing color boxes for unchecked calendars

>+    for (var i = 0; i < gCalendars.length; ++i) {
>+        updateLtnStyleSheet(gCalendars[i]);
>+    }

You might want to do something like:
for each (var cal in gCalendars) {
    updateLtnStyleSheet(cal);
}
(not tested for syntax errors etc)

r=mvl with that.

Comment 15

[Thomas Benisch]

(In reply to comment #14)
> You might want to do something like:
> for each (var cal in gCalendars) {
>     updateLtnStyleSheet(cal);
> }
> (not tested for syntax errors etc)

done

Comment 16

[Thomas Benisch]

patch checked in on HEAD and MOZILLA_1_8_BRANCH

Comment 17

[Stefan Sitter [:ssitter]]

Verified with Lightning/0.5pre (2007032003) and Thunderbird/1.5.0.10 (20070221)