Closed Bug 1472301 Opened 3 years ago Closed 3 years ago

Port bug 1400266 to TB: Use SVG context paint for Linux tree twisties

Categories

(Thunderbird :: Theme, enhancement)

All
Linux
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 63.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(4 files)

Bug 1400266 implements the SVG twisties like bug 1466826 did for Windows. We need to follow and use the SVG icons instead the PNG icons.
Bug 1400266 is in M-I and lands with one of the next merges.

This the same as bug 1470740 but for Linux. This patch makes now the chat.css and the imAccountWizard.css much simpler because we distinguish now only between Mac and Linux/Windows in the preprocessing.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8988869 - Flags: review?(jorgk)
Comment on attachment 8988869 [details] [diff] [review]
Linux-twisty.patch

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

rs=jorgk. I'm not a Calendar peer, but Calendar is broken, so we can't test this. I don't know whether Linux and Mac still work, Windows does. Code looks OK.

::: calendar/base/themes/linux/widgets/calendar-widgets.css
@@ +16,4 @@
>  }
>  
>  treenode-checkbox[checked="true"] > .checkbox-check {
> +  list-style-image: url("chrome://global/skin/tree/twisty-expanded.svg");

Why does background become list-style?

@@ +18,5 @@
>  treenode-checkbox[checked="true"] > .checkbox-check {
> +  list-style-image: url("chrome://global/skin/tree/twisty-expanded.svg");
> +}
> +
> +treenode-checkbox:-moz-locale-dir(rtl) > .checkbox-check {

Where does this rule come from? Added bonus for RTL?
Attachment #8988869 - Flags: review?(jorgk) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/7d3a1ddf8dd6
Port bug 1400266 to TB: Use SVG context paint for Linux tree twisties. rs=jorgk
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 63.0
Comment on attachment 8988869 [details] [diff] [review]
Linux-twisty.patch

I landed this in a hurry. Aceman, can you please check Linux.
Attachment #8988869 - Flags: feedback?(acelists)
(In reply to Jorg K (GMT+2) from comment #2)
> Comment on attachment 8988869 [details] [diff] [review]
> Linux-twisty.patch
> 
> Review of attachment 8988869 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> rs=jorgk. I'm not a Calendar peer, but Calendar is broken, so we can't test
> this. I don't know whether Linux and Mac still work, Windows does. Code
> looks OK.
> 
> ::: calendar/base/themes/linux/widgets/calendar-widgets.css
> @@ +16,4 @@
> >  }
> >  
> >  treenode-checkbox[checked="true"] > .checkbox-check {
> > +  list-style-image: url("chrome://global/skin/tree/twisty-expanded.svg");
> 
> Why does background become list-style?

Copy/paste error. Changed to background-image. It's hard to test when calendar doesn't work.

> @@ +18,5 @@
> >  treenode-checkbox[checked="true"] > .checkbox-check {
> > +  list-style-image: url("chrome://global/skin/tree/twisty-expanded.svg");
> > +}
> > +
> > +treenode-checkbox:-moz-locale-dir(rtl) > .checkbox-check {
> 
> Where does this rule come from? Added bonus for RTL?

Yes, we have the ability, so we should use it. It's a copy from Windows.
Attachment #8988978 - Flags: review?(jorgk)
Comment on attachment 8988978 [details] [diff] [review]
Bug1472301-fup.patch

Thanks.
Attachment #8988978 - Flags: review?(jorgk) → review+
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/85eff6f1dec8
Follow-up: use background-image instead of list-style-image. r=jorgk DONTBUILD
Comment on attachment 8988869 [details] [diff] [review]
Linux-twisty.patch

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

Thanks, twisties in folder pane look OK.
Attachment #8988869 - Flags: feedback?(acelists) → feedback+
Wrong answer ;-( - This is about the twisties in chat: 1) Left under Conversations 2) Right under Previous Conversations and 3) on the Advanced Options panel when creating a new chat account using the wizard. Please check again.
Comment on attachment 8988869 [details] [diff] [review]
Linux-twisty.patch

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

Thanks.
The patch also touched folder pane css so I tested that.
Twisties in Previous conversations look OK.

I see no twisties on the left at Conversations.
Yes, Conversations can be collapsed/expanded but there is no indication you can do that. So the twisty may be missing.

Advanced option in account creation do miss twisties. There is also a label like 'IRC options' bot no twisty so it can't be seen you can expand the section.
Attachment #8988869 - Flags: feedback+ → feedback-
Attached image chat.png
This is from today Daily with the changes included. It looks good for me.
Attached image accountWizzard.png
And this the account wizzard with the same Daily.

I see no problems.
Right, I had to update m-c too to get the new twisties. I can't say I like the new appearance, but whatever.

But in the Advanced options the twisties do not twist, they do not change the direction when clicked/when their section is expanded/collapsed.
(In reply to :aceman from comment #13)
> But in the Advanced options the twisties do not twist, they do not change
> the direction when clicked/when their section is expanded/collapsed.

That's bug 1472442.
Comment on attachment 8988869 [details] [diff] [review]
Linux-twisty.patch

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

Thanks.
Attachment #8988869 - Flags: feedback- → feedback+
You need to log in before you can comment on or make changes to this bug.