Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: lib/icon.js

Issue 29334223: Issue 3532 - Generate animation images at runtime (Closed)
Patch Set: Remove stopAnimation closure and hard coded opacity range Created Jan. 26, 2016, 6:43 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/ext/background.js ('k') | lib/notificationHelper.js » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/icon.js
diff --git a/lib/icon.js b/lib/icon.js
index 2011d1e23701aada50df48f3f2722514d3b3b24e..86547d610f9b7f22d7037376bb73dd87b5e255f7 100644
--- a/lib/icon.js
+++ b/lib/icon.js
@@ -17,73 +17,140 @@
/** @module icon */
-const numberOfFrames = 10;
+"use strict";
-let whitelistedState = new ext.PageMap();
-let notificationType = null;
+const frameOpacities = [0.0, 0.1, 0.2, 0.3, 0.4, 0.5, 0.6, 0.7, 0.8, 0.9,
+ 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0, 1.0,
+ 0.9, 0.8, 0.7, 0.6, 0.5, 0.4, 0.3, 0.2, 0.1, 0.0];
+const numberOfFrames = frameOpacities.length;
+const safariPlatform = require("info").platform == "safari";
+
+let frameInterval = null;
let animationInterval = null;
-let animationStep = 0;
+let whitelistedState = new ext.PageMap();
-function getIconPath(whitelisted)
+function loadImage(url)
{
- let filename = "icons/abp-$size";
+ return new Promise((resolve, reject) => {
+ let image = new Image();
+ image.src = url;
+ image.addEventListener("load", () =>
Sebastian Noack 2016/01/26 22:35:15 Nit: The indentation of the block below is inconsi
kzar 2016/01/26 22:59:31 Done.
+ {
+ resolve(image);
+ });
+ image.addEventListener("error", () => {
+ reject("Failed to load image " + url);
+ });
+ });
+};
- // If the current page is whitelisted, pick an icon that indicates that
- // Adblock Plus is disabled, however not when the notification icon has
- // full opacity, or on Safari where icons are genrally grayscale-only.
- if (whitelisted && animationStep < numberOfFrames && require("info").platform != "safari")
- filename += "-whitelisted";
+function setIcon(page, notificationType, opacity, frames)
+{
+ opacity = opacity || 0;
+ let whitelisted = !!whitelistedState.get(page) && !safariPlatform;
- // If the icon is currently animating to indicate a pending notification,
- // pick the icon for the corresponing notification type and animation frame.
- if (notificationType && animationStep > 0)
+ if (!notificationType || !frames)
{
- filename += "-notification-" + notificationType;
-
- if (animationStep < numberOfFrames)
- filename += "-" + animationStep;
+ if (notificationType && opacity > 0.5)
Sebastian Noack 2016/01/26 22:35:17 The check for notificationType seems to be redunda
kzar 2016/01/26 22:59:32 Done.
+ page.browserAction.setIcon("icons/abp-$size-notification-"
+ + notificationType + ".png");
+ else
+ page.browserAction.setIcon("icons/abp-$size" +
+ (whitelisted ? "-whitelisted" : "") + ".png");
+ }
+ else
+ {
+ chrome.browserAction.setIcon({
+ tabId: page._id,
+ imageData: frames["" + opacity + whitelisted]
+ });
Sebastian Noack 2016/01/26 22:35:15 Nit: You should close the braces on the same depth
kzar 2016/01/26 22:59:32 Done.
}
-
- return filename + ".png";
}
-function setIcon(page)
+function renderFrames(notificationType)
{
- page.browserAction.setIcon(getIconPath(whitelistedState.get(page)));
+ if (safariPlatform)
+ return Promise.resolve(null);
+
+ return Promise.all([
+ loadImage("icons/abp-19.png"),
+ loadImage("icons/abp-19-whitelisted.png"),
+ loadImage("icons/abp-19-notification-" + notificationType + ".png"),
+ loadImage("icons/abp-38.png"),
+ loadImage("icons/abp-38-whitelisted.png"),
+ loadImage("icons/abp-38-notification-" + notificationType + ".png"),
+ ]).then(images =>
+ {
+ let images = {
Sebastian Noack 2016/01/26 22:35:15 Nit: You declare a variable that is already define
kzar 2016/01/26 22:59:33 I think it's pretty readable in this instance.
+ 19: { base: [images[0], images[1]], overlay: images[2] },
+ 38: { base: [images[3], images[4]], overlay: images[5] }
+ };
+
+ let frames = {};
+ let canvas = document.createElement("canvas");
+ let context = canvas.getContext("2d");
+
+ for (let whitelisted of [false, true])
+ {
+ for (let i = 0, opacity = 0; i <= 10; opacity = ++i / 10)
+ {
+ let imageData = {};
+ for (let size of [19, 38])
+ {
+ canvas.width = size;
+ canvas.height = size;
+ context.globalAlpha = 1;
+ context.drawImage(images[size]["base"][whitelisted | 0], 0, 0);
+ context.globalAlpha = opacity;
+ context.drawImage(images[size]["overlay"], 0, 0);
+ imageData[size] = context.getImageData(0, 0, size, size);
+ }
+ frames["" + opacity + whitelisted] = imageData;
+ }
+ }
+
+ return frames;
+ });
}
-function runAnimation()
+function runAnimation(notificationType)
{
- return setInterval(function()
+ function playAnimation(frames)
{
- ext.pages.query({active: true}, function(pages)
+ let animationStep = 0;
+ ext.pages.query({active: true}, pages =>
{
- let fadeInInterval = setInterval(function()
+ function appendActivePage(page)
{
- animationStep++;
- pages.forEach(setIcon);
+ pages.push(page);
+ }
+ ext.pages.onActivated.addListener(appendActivePage);
- if (animationStep < numberOfFrames)
- return;
+ frameInterval = setInterval(() =>
+ {
+ let opacity = frameOpacities[animationStep++];
+ pages.forEach(page => {
Sebastian Noack 2016/01/26 22:35:16 Nit: Why not a for-of loop?
kzar 2016/01/26 22:59:33 Done.
+ if (whitelistedState.has(page))
+ setIcon(page, notificationType, opacity, frames);
+ });
- setTimeout(function()
+ if (animationStep > numberOfFrames)
{
- let fadeOutInterval = setInterval(function()
- {
- animationStep--;
- pages.forEach(setIcon);
-
- if (animationStep > 0)
- return;
-
- clearInterval(fadeOutInterval);
- }, 100);
- },1000);
-
- clearInterval(fadeInInterval);
+ animationStep = 0;
+ clearInterval(frameInterval);
+ frameInterval = null;
+ ext.pages.onActivated.removeListener(appendActivePage);
+ }
}, 100);
});
- }, 10000);
+ }
+
+ renderFrames(notificationType).then(frames =>
+ {
+ playAnimation(frames);
+ animationInterval = setInterval(() => { playAnimation(frames); },
+ 10000);
Sebastian Noack 2016/01/26 22:35:16 Nit: Wrapping the line here doesn't seem to be nec
kzar 2016/01/26 22:59:31 Done.
+ });
}
/**
@@ -95,33 +162,31 @@ function runAnimation()
*/
exports.updateIcon = function(page, whitelisted)
{
- page.browserAction.setIcon(getIconPath(whitelisted));
whitelistedState.set(page, whitelisted);
+ if (frameInterval == null)
+ setIcon(page);
};
-/**
- * Starts to animate the browser action icon to indicate a pending notifcation.
- *
- * @param {string} type The notification type (i.e: "information" or "critical")
- */
-exports.startIconAnimation = function(type)
-{
- notificationType = type;
-
- if (animationInterval == null)
- animationInterval = runAnimation();
-};
-
+let stopIconAnimation =
/**
* Stops to animate the browser action icon.
*/
exports.stopIconAnimation = function()
{
+ if (frameInterval != null)
+ clearInterval(frameInterval);
if (animationInterval != null)
- {
clearInterval(animationInterval);
- animationInterval = null;
- }
+ frameInterval = animationInterval = null;
+};
- notificationType = null;
+/**
+ * Starts to animate the browser action icon to indicate a pending notifcation.
+ *
+ * @param {string} type The notification type (i.e: "information" or "critical")
+ */
+exports.startIconAnimation = function(type)
+{
+ stopIconAnimation();
+ runAnimation(type);
};
« no previous file with comments | « chrome/ext/background.js ('k') | lib/notificationHelper.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld