"Other month" day label in month view have a different color

RESOLVED FIXED in 4.0.0.1

Status

Calendar
Calendar Views
--
trivial
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: Decathlon, Assigned: Decathlon)

Tracking

Trunk
4.0.0.1

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Assignee)

Description

3 years ago
In month view, the font for the day labels inside the day boxes that belong to the other month have a darker color.
(Assignee)

Comment 1

3 years ago
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.
Attachment #8548748 - Flags: review?(richard.marti)
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).
Attachment #8548748 - Flags: review?(richard.marti) → review-
(Assignee)

Comment 3

3 years ago
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?
Attachment #8548748 - Attachment is obsolete: true
Attachment #8549949 - Flags: review?(richard.marti)
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.
Attachment #8549949 - Flags: review?(richard.marti) → review+
(Assignee)

Comment 5

3 years ago
Created attachment 8553665 [details] [diff] [review]
patch-v2 for checkin
Attachment #8549949 - Attachment is obsolete: true
Attachment #8553665 - Flags: review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
Pushed to comm-central changeset 2553a8e486b5
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → 4.0
You need to log in before you can comment on or make changes to this bug.