Opened on 02/06/2015 at 02:00:07 PM
Closed on 02/13/2015 at 11:56:14 AM
#1965 closed change (fixed)
Refactor icon animation code
| Reported by: | sebastian | Assignee: | sebastian |
|---|---|---|---|
| Priority: | P4 | Milestone: | Adblock-Plus-1.8.11-for-Chrome-Opera-Safari |
| Module: | Platform | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Platform: | Unknown | |
| Ready: | yes | Confidential: | no |
| Tester: | Verified working: | ||
| Review URL(s): | |||
Description (last modified by sebastian)
Background
There are currently some issues with the approach of the code animating the icon to indicate notifications.
What to change
The logic to generate the icon's filename is currently spread into two places, where the code handling the icon animation injects the -notification-<type>-<frame> suffix with a regex. This is a rather ugly hack. Moreover, it breaks when generating the whitelisted icon overlayed with a notification icon at full opacity. A better approach would be to combine the logic to generate the icon's filename into a single function, using simple string concatenation.
Also the icon animation is currently implemented with a global object. So while on it, we should make it a proper module, to improve encapsulation and to makes things more consistent.
Hints for testers
In order to test the icon animation (after this changes has been merged), go to chrome://extensions, activate "Developer Mode", inspect the extension's background page, and run startIconAnimation("critical").
Attachments (0)
Change History (4)
comment:1 Changed on 02/06/2015 at 02:00:22 PM by sebastian
- Summary changed from Rafactor icon animation code to Refactor icon animation code
comment:2 Changed on 02/06/2015 at 02:09:21 PM by sebastian
- Review URL(s) modified (diff)
- Status changed from new to reviewing
comment:4 Changed on 02/13/2015 at 11:56:14 AM by sebastian
- Milestone set to Adblock-Plus-1.8.11-for-Chrome-Opera-Safari
- Resolution set to fixed
- Status changed from reviewing to closed
https://hg.adblockplus.org/adblockpluschrome/rev/cd1af5aaa69f