Icons in task view and mail icon in "Write" button are size scaled with aero theme.

RESOLVED FIXED in 1.0

Status

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

People

(Reporter: Decathlon, Assigned: Decathlon)

Tracking

Lightning 1.0b7
x86_64
Windows 7

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
There are a few trivial issues about icons in the winstripe aero theme.

- the monochrome icons in the toolbar buttons (task view) are scaled to 16X16 instead of the natural size 18x18;
- the same happens for the icon related to the mail item in the write button menu for the Thunderbird's main toolbar (maybe the same happens for the Mac, but I can't verify that);
- the day number text in the todaypane button needs to be moved to the left
- in task view, the position of the toolbar buttons need a little adjustment to make the same as the message-pane in mail mode (not aero theme specific).
(Assignee)

Comment 1

6 years ago
Created attachment 562411 [details] [diff] [review]
patch - v1

Icons for aero theme are 18x18, instead the others are 16x16, so I've used "width: auto" and "height: auto" directly in the file calendar-task-view.css that should cover both cases. 

For what I've understood, the aero theme is applied on Windows Vista/Seven even if the selected theme is different, e.g. the Windows Classic theme (and here Richard, I ask you a confirm because you know better than me). 
In this case the mail icon in the "Write" button (main toolbar) needs the mail-toolbar-small.png image (16x16) for Win XP (file lightning.css), instead the aero theme needs the 18x18 monochrome mail icon. Again "width: auto" and "height: auto" in lightning.css should cover both cases.

I also delete the rule |#newMsgButton-mail-menuitem .toolbarbutton-icon| because it seems useless, please Richard, correct me if I'm wrong.

For the todaypane button, I've deleted the code that I asked you to add in bug 668958. Don't know why, but without that, on Miramar the text was displaced, instead, on TB 7/8 the day number is correctly positioned only if that code is deleted. Could you confirm?
Attachment #562411 - Flags: review?(richard.marti)
Comment on attachment 562411 [details] [diff] [review]
patch - v1

All in all this patch looks good.
But some comments:

+#task-actions-toolbox .toolbarbutton-icon {
+    margin: 0 !important;
+    width: auto !important;
+    height: auto !important;

The three !important aren't needed. It's the last rule applied and no !important in previous rules needs to be overridden.

Instead of adding

+#newMsgButton-mail-menuitem  .menu-iconic-icon {
+  margin: 0 !important;
+  width: auto !important;
+  height: auto !important;
+}

in lightning.css, I would propose to change in lightning-aero.css this:

#newMsgButton-mail-menuitem {
  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar.png);
  -moz-image-region: rect(0px 36px 18px 18px) !important;

to:

#newMsgButton-mail-menuitem {
  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar.png);
  -moz-image-region: rect(1px 35px 17px 19px) !important;
}

(and still remove #newMsgButton-mail-menuitem .toolbarbutton-icon { rule as in your patch). This because you also corrected in this patch the icon for XP to 16px and with my proposed change the icon is also 16px for Vista/Win7. So the margin: 0 and width/height: auto aren't needed any more. If you still want to leave it, again the !important aren't needed and between #newMsgButton-mail-menuitem and .menu-iconic-icon is a space to much.

> I also delete the rule |#newMsgButton-mail-menuitem .toolbarbutton-icon|
> because it seems useless, please Richard, correct me if I'm wrong.

That's correct, in my patch for Bug 684518 I have it also removed.

> For the todaypane button, I've deleted the code that I asked you to add in
> bug 668958. Don't know why, but without that, on Miramar the text was
> displaced, instead, on TB 7/8 the day number is correctly positioned only if
> that code is deleted. Could you confirm?

I can also confirm this.

r+ when you check my comments.
Attachment #562411 - Flags: review?(richard.marti) → review+
(Assignee)

Comment 3

6 years ago
Created attachment 564330 [details] [diff] [review]
patch - v2

All notes in previous comment addressed.
I had forgotten the pinstripe and gnomestripe themes for the file calendar-task-view.css. I merely copied and pasted the code from winstripe because I can't test on Linux and Mac. Could you please verify that the buttons in the task toolbar get the correct position on those platforms? The review it's only for this issue.
Attachment #562411 - Attachment is obsolete: true
Attachment #564330 - Flags: review?(richard.marti)
Comment on attachment 564330 [details] [diff] [review]
patch - v2

The Windows part looks good.

Can you also add on Linux in lightning.css this part

+  list-style-image: url(chrome://messenger/skin/icons/mail-toolbar-small.png);
+  -moz-image-region: rect(0px 32px 16px 16px);

On Mac this isn't possible because the small toolbarbuttons have 24px icons.

For Mac and Linux the #other-actions-box needs a -moz-margin-end: 3px; instead of a -moz-margin-end: -2px; (see image I'll attach).

Also for both. This isn't needed but doesn't harm if you leave it

+#task-actions-toolbox .toolbarbutton-icon {
+    margin: 0;
+    width: auto;
+    height: auto;

because where are only 16px icons used and no mix with 18px icons.
Attachment #564330 - Flags: review?(richard.marti) → review-
Created attachment 565184 [details]
Comparison between mail header buttons and Task buttons with patch applied under Linux

The gap between Task buttons and border is closer than in mail part.

I see also a missing right border and wrong button text colors. But this isn't part of this bug. I'll file a new for this issues.
Update for Linux:

Bug 673826 changed with the today nightly the padding-end of the toolbox from 6px to 2px. This means for Linux you should now use for the #other-actions-box a -moz-margin-end: -1px;

Mac wasn't changed. So the -moz-margin-end: 3px; is still valid.
(Assignee)

Comment 7

6 years ago
Created attachment 568638 [details] [diff] [review]
patch - v3

(In reply to Richard Marti [:paenglab] from comment #4)
> 
> Can you also add on Linux in lightning.css this part
> 
> +  list-style-image:
> url(chrome://messenger/skin/icons/mail-toolbar-small.png);
> +  -moz-image-region: rect(0px 32px 16px 16px);
> 

Done.

> 
> Also for both. This isn't needed but doesn't harm if you leave it
> 
> +#task-actions-toolbox .toolbarbutton-icon {
> +    margin: 0;
> +    width: auto;
> +    height: auto;
> 
> because where are only 16px icons used and no mix with 18px icons.

I deleted them both on pinstripe and gnomestripe.

(In reply to Richard Marti [:paenglab] from comment #6)
> ... This means for Linux you should now use for the
> #other-actions-box a -moz-margin-end: -1px;
> Mac wasn't changed. So the -moz-margin-end: 3px; is still valid.

Done.

If pinstripe or gnomestripe need some adjustment, please feel free to make them since I can't test.
Attachment #564330 - Attachment is obsolete: true
Attachment #568638 - Flags: review?(richard.marti)
Status: NEW → ASSIGNED
Comment on attachment 568638 [details] [diff] [review]
patch - v3

(In reply to Decathlon from comment #7)
> If pinstripe or gnomestripe need some adjustment, please feel free to make
> them since I can't test.

Not needed. Looks good.

r+
Attachment #568638 - Flags: review?(richard.marti) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
comm-central changeset 2b890c7316e8
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Trunk
Backported to releases/comm-aurora changeset 672ee035ff4a
Target Milestone: Trunk → 1.0b9
Backported to releases/comm-beta changeset a5f8a5093dae
Target Milestone: 1.0b9 → 1.0b8
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.