Last Comment Bug 1121373 - "Other month" day label in month view have a different color
: "Other month" day label in month view have a different color
Status: RESOLVED FIXED
:
Product: Calendar
Classification: Client Software
Component: Calendar Views (show other bugs)
: Trunk
: All All
-- trivial (vote)
: 4.0.0.1
Assigned To: Decathlon
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2015-01-14 01:16 PST by Decathlon
Modified: 2015-01-27 16:23 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch - v1 (1.53 KB, patch)
2015-01-14 01:20 PST, Decathlon
richard.marti: review-
Details | Diff | Splinter Review
patch - v2 (2.31 KB, patch)
2015-01-15 15:54 PST, Decathlon
richard.marti: review+
Details | Diff | Splinter Review
patch-v2 for checkin (2.31 KB, patch)
2015-01-23 02:55 PST, Decathlon
bv1578: review+
Details | Diff | Splinter Review

Description User image Decathlon 2015-01-14 01:16:56 PST
In month view, the font for the day labels inside the day boxes that belong to the other month have a darker color.
Comment 1 User image Decathlon 2015-01-14 01:20:18 PST
Created attachment 8548748 [details] [diff] [review]
patch - v1

The patch should fix.
I've also added a rule about the background color of the label when the day is "other-month", "day-off" and selected that seems missing since ever.
Comment 2 User image Richard Marti (:Paenglab) 2015-01-14 12:03:55 PST
Comment on attachment 8548748 [details] [diff] [review]
patch - v1

Review of attachment 8548748 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, --viewMonthDayOtherColor is not really needed.

r- now, because of the two comments.

::: calendar/base/themes/common/calendar-views.css
@@ +601,1 @@
>      background-color: var(--viewMonthDayOtherLabelBackground);

When you remove the var here, please check if this var is still used. Actually --viewMonthDayOtherColor is no more used and you can also remove the variable definitions.

@@ +632,1 @@
>      background-color: var(--viewMonthDayBoxSelectedBackground);

Please don't remove this variable as for the systemcolors it needs the correct color to match the background color. Linux has on some themes weird highlight background colors and without the matching text color it can be unreadable.

Instead of removing the variable please set the color of --viewMonthDayBoxLabelColor for the --viewMonthDayBoxSelectedColor in normal color variable set (#616163).
Comment 3 User image Decathlon 2015-01-15 15:54:33 PST
Created attachment 8549949 [details] [diff] [review]
patch - v2

I can't verify the part about Linux, is it correct in this way?

I've also noticed that the background color of the label for "today" when systemcolors are active is different than before the introduction of the css variables (now is Highlight, before it was absent).
Looks like it needs a different variable for the background in the rule
.calendar-month-day-box-date-label[relation="today"][selected="true"]
in order to have the possibility to differentiate the background in the systemcolors case.
What do you think?
Comment 4 User image Richard Marti (:Paenglab) 2015-01-16 12:00:12 PST
Comment on attachment 8549949 [details] [diff] [review]
patch - v2

Looks good now. r+

(In reply to Decathlon from comment #3)
> 
> I've also noticed that the background color of the label for "today" when
> systemcolors are active is different than before the introduction of the css
> variables (now is Highlight, before it was absent).
> Looks like it needs a different variable for the background in the rule
> .calendar-month-day-box-date-label[relation="today"][selected="true"]
> in order to have the possibility to differentiate the background in the
> systemcolors case.
> What do you think?

Good catch, before, systemcolors has set the color and now it's the background color. I file a bug for this after this bug's landing.
Comment 5 User image Decathlon 2015-01-23 02:55:35 PST
Created attachment 8553665 [details] [diff] [review]
patch-v2 for checkin
Comment 6 User image Philipp Kewisch [:Fallen] 2015-01-27 16:23:03 PST
Pushed to comm-central changeset 2553a8e486b5

Note You need to log in before you can comment on or make changes to this bug.