Closed Bug 954690 Opened 8 years ago Closed 8 years ago

Replace JS animation code in messagestyle themes by CSS

Categories

(Instantbird :: Other, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: benediktp, Assigned: benediktp)

References

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 1259 at 2012-02-09 08:47:00 UTC ***

The dark theme animates the opacity of elements using JS and setTimeout calls. This is bad since we could use a CSS animation instead.

The section in question is in here (smoothDisplay) and is at least used by system messages.

http://lxr.instantbird.org/instantbird/source/instantbird/themes/messages/dark/Footer.html
Blocks: 953941
*** Original post on bio 1259 at 2012-02-10 07:51:12 UTC ***

The "Paper Sheets" messagestyle theme suffers from the same problem. I'm extending the scope of this bug to cover this too.

"Bubbles" was fixed already by bug 954429 (bio 995) and "Simple" doesn't include any JS code.
Summary: Dark theme: replace JS animation code with CSS animation → Replace JS animation code in messagestyle themes by CSS
Attached patch Patch v1 (obsolete) — Splinter Review
*** Original post on bio 1259 as attmnt 1167 at 2012-02-10 08:09:00 UTC ***

This patch removes the JS animation code ("smoothDisplay") and replaces it with a CSS transition in both of the themes.

I can change the alignment of the -moz-prefixed values if you don't like it. It looked cleaner and less distracting to me than with the full indentation.

I also removed some trailing blanks in one of the files.
Attachment #8352913 - Flags: review?(florian)
Assignee: nobody → benediktp
Status: NEW → ASSIGNED
Comment on attachment 8352913 [details] [diff] [review]
Patch v1

*** Original change on bio 1259 attmnt 1167 at 2012-02-12 15:10:54 UTC ***

First, thank you very much for working on this nice cleanup!

>diff --git a/instantbird/themes/messages/papersheets/main.css b/instantbird/themes/messages/papersheets/main.css

>+p:hover span.date-next {
>+  opacity: 1;
> }

You wanted a child selector here rather than a descendant selector, didn't you?
(https://developer.mozilla.org/en/Writing_Efficient_CSS#Avoid_the_descendant_selector)

Lots of other CSS selectors are inefficient in this file (selecting both the tag name and the class where only the class would be needed), and we should clean up these files in the future, but for now I think we should avoid adding more inefficiencies :).

(In reply to comment #2)

> I can change the alignment of the -moz-prefixed values if you don't like it.

I don't like it, please add back the missing space.
Attachment #8352913 - Flags: review?(florian) → review-
Attached patch Patch v2Splinter Review
*** Original post on bio 1259 as attmnt 1197 at 2012-02-25 15:21:00 UTC ***

Patch adressing review comments.
Attachment #8352948 - Flags: review?(florian)
Comment on attachment 8352913 [details] [diff] [review]
Patch v1

*** Original change on bio 1259 attmnt 1167 at 2012-02-25 15:21:01 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352913 - Attachment is obsolete: true
Comment on attachment 8352948 [details] [diff] [review]
Patch v2

*** Original change on bio 1259 attmnt 1197 at 2012-04-30 13:42:23 UTC ***

Looks ok, although I haven't tried it. Sorry for the delay, and thanks again for working on this! :-)
Attachment #8352948 - Flags: review?(florian) → review+
*** Original post on bio 1259 at 2012-05-01 10:59:45 UTC ***

https://hg.instantbird.org/instantbird/rev/44cdcdd49d40

I added a space before your name in the license header of the papersheets main.css file.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
You need to log in before you can comment on or make changes to this bug.