Closed Bug 368558 Opened 13 years ago Closed 13 years ago

add colors to Lightning calendars tab

Categories

(Calendar :: Lightning Only, defect)

x86
Windows XP
defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: thomas.benisch, Assigned: thomas.benisch)

References

Details

Attachments

(3 files, 1 obsolete file)

The calendar colors are not displayed in the Lightning calendars tab.
This is a major usability issue.
Attached patch calendar colors (obsolete) — Splinter Review
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.
Assignee: nobody → thomas.benisch
Status: NEW → ASSIGNED
Attachment #253168 - Flags: ui-review?(mvl)
Attachment #253168 - Flags: first-review?(lilmatt)
This is a screenshot of the calendars tab with color boxes added.
See bug 298360 for the same issue.
(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?
I'd dupe it the other way around.  This bug has more recent patches (read: less bit-rot)
Comment on attachment 253168 [details] [diff] [review]
calendar colors

ui-review=mvl
Attachment #253168 - Flags: ui-review?(mvl) → ui-review+
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.
Attachment #253168 - Flags: first-review?(lilmatt) → first-review+
This patch fixes all remarks from comment #7.
I carried over the ui-review+ and first-review+ flags
from the last patch.
Attachment #253168 - Attachment is obsolete: true
Attachment #253314 - Flags: ui-review+
Attachment #253314 - Flags: second-review?(mvl)
Attachment #253314 - Flags: first-review+
Duplicate of this bug: 298360
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
Attachment #253314 - Flags: second-review?(mvl) → second-review+
Thanks for those fast reviews.

Patch checked in on HEAD and MOZILLA_1_8_BRANCH.
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
This patch fixes the problem with the missing color boxes
for unchecked calendars after application startup.
Attachment #258541 - Flags: first-review?(mvl)
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.
Attachment #258541 - Flags: first-review?(mvl) → first-review+
(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
patch checked in on HEAD and MOZILLA_1_8_BRANCH
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Verified with Lightning/0.5pre (2007032003) and Thunderbird/1.5.0.10 (20070221)
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.