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

Side by Side Diff: notification.js

Issue 29567749: Issue 5593 - Use messaging for the popup's notification code (Closed)
Patch Set: Replace !== with !=, === with == Created Oct. 8, 2017, 10:11 a.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « dependencies ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details. 12 * GNU General Public License for more details.
13 * 13 *
14 * You should have received a copy of the GNU General Public License 14 * You should have received a copy of the GNU General Public License
15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
16 */ 16 */
17 17
18 /* global togglePref */
Sebastian Noack 2017/10/08 19:17:22 As I already told Manish on a different review, I'
kzar 2017/10/09 10:33:25 Acknowledged.
19
18 "use strict"; 20 "use strict";
19 21
20 const {require} = ext.backgroundPage.getWindow();
21
22 const {Utils} = require("utils");
23 const {Notification} = require("notification");
24 const {getActiveNotification, shouldDisplay} = require("notificationHelper");
25
26 function getDocLinks(notification) 22 function getDocLinks(notification)
27 { 23 {
24 let docLinks = [];
Manish Jethani 2017/10/08 22:31:34 The variable is unnecessary here.
kzar 2017/10/09 10:33:25 Sure, we could return an empty array if notificati
Manish Jethani 2017/10/09 10:54:12 I must be looking at the wrong diff, because to me
kzar 2017/10/09 15:10:54 Oh yea, you are right sorry. I switched to mapping
25
28 if (!notification.links) 26 if (!notification.links)
29 return []; 27 return Promise.resolve(docLinks);
30 28
31 let docLinks = []; 29 return Promise.all(
32 notification.links.forEach(link => 30 notification.links.map(link =>
33 { 31 {
34 docLinks.push(Utils.getDocLink(link)); 32 return new Promise((resolve, reject) =>
Manish Jethani 2017/10/08 22:31:34 If you prefer it, you could lose the braces and th
kzar 2017/10/09 10:33:25 Yea I considered that, but I figured it looked nic
Manish Jethani 2017/10/09 10:54:12 Acknowledged.
35 }); 33 {
36 return docLinks; 34 chrome.runtime.sendMessage({
35 type: "app.get",
36 what: "doclink",
37 link
38 }, resolve);
39 });
40 })
41 );
37 } 42 }
38 43
39 function insertMessage(element, text, links) 44 function insertMessage(element, text, links)
40 { 45 {
41 let match = /^(.*?)<(a|strong)>(.*?)<\/\2>(.*)$/.exec(text); 46 let match = /^(.*?)<(a|strong)>(.*?)<\/\2>(.*)$/.exec(text);
42 if (!match) 47 if (!match)
43 { 48 {
44 element.appendChild(document.createTextNode(text)); 49 element.appendChild(document.createTextNode(text));
45 return; 50 return;
46 } 51 }
47 52
48 let before = match[1]; 53 let before = match[1];
49 let tagName = match[2]; 54 let tagName = match[2];
50 let value = match[3]; 55 let value = match[3];
51 let after = match[4]; 56 let after = match[4];
52 57
53 insertMessage(element, before, links); 58 insertMessage(element, before, links);
54 59
55 let newElement = document.createElement(tagName); 60 let newElement = document.createElement(tagName);
56 if (tagName === "a" && links && links.length) 61 if (tagName == "a" && links && links.length)
57 newElement.href = links.shift(); 62 newElement.href = links.shift();
58 insertMessage(newElement, value, links); 63 insertMessage(newElement, value, links);
59 element.appendChild(newElement); 64 element.appendChild(newElement);
60 65
61 insertMessage(element, after, links); 66 insertMessage(element, after, links);
62 } 67 }
63 68
64 window.addEventListener("load", () => 69 window.addEventListener("load", () =>
65 { 70 {
66 let notification = getActiveNotification(); 71 chrome.runtime.sendMessage({
67 if (!notification || !shouldDisplay("popup", notification.type)) 72 type: "notifications.get",
68 return; 73 displayMethod: "popup"
74 }, notification =>
75 {
76 if (!notification)
77 return;
69 78
70 let texts = Notification.getLocalizedTexts(notification); 79 let titleElement = document.getElementById("notification-title");
71 let titleElement = document.getElementById("notification-title"); 80 let messageElement = document.getElementById("notification-message");
72 titleElement.textContent = texts.title;
73 81
74 let docLinks = getDocLinks(notification); 82 titleElement.textContent = notification.texts.title;
75 let messageElement = document.getElementById("notification-message");
76 insertMessage(messageElement, texts.message, docLinks);
77 83
78 messageElement.addEventListener("click", event => 84 getDocLinks(notification).then(docLinks =>
79 { 85 {
80 let link = event.target; 86 insertMessage(messageElement, notification.texts.message, docLinks);
81 while (link && link !== messageElement && link.localName !== "a") 87
82 link = link.parentNode; 88 messageElement.addEventListener("click", event =>
83 if (!link) 89 {
84 return; 90 let link = event.target;
85 event.preventDefault(); 91 while (link && link != messageElement && link.localName != "a")
86 event.stopPropagation(); 92 link = link.parentNode;
87 ext.pages.open(link.href); 93 if (!link)
94 return;
95 event.preventDefault();
96 event.stopPropagation();
97 ext.pages.open(link.href);
98 });
99 });
100
101 let notificationElement = document.getElementById("notification");
102 notificationElement.className = notification.type;
103 notificationElement.hidden = false;
104 notificationElement.addEventListener("click", event =>
105 {
106 if (event.target.id == "notification-close")
107 notificationElement.classList.add("closing");
108 else if (event.target.id == "notification-optout" ||
109 event.target.id == "notification-hide")
110 {
111 if (event.target.id == "notification-optout")
112 togglePref("notifications_ignoredcategories");
Manish Jethani 2017/10/08 22:31:34 The "prefs.toggle" implementation in messageRespon
Manish Jethani 2017/10/08 22:40:59 Since we're adding "notifications.get", we could a
kzar 2017/10/09 10:33:25 Well my understanding was the option would be togg
Manish Jethani 2017/10/09 10:54:12 The reason the original code was passing true as t
Manish Jethani 2017/10/09 11:53:52 Looking at this code in adblockpluscore lib/notifi
kzar 2017/10/09 15:10:54 Done.
113
114 notificationElement.hidden = true;
115 notification.onClicked();
116 }
117 }, true);
88 }); 118 });
89
90 let notificationElement = document.getElementById("notification");
91 notificationElement.className = notification.type;
92 notificationElement.hidden = false;
93 notificationElement.addEventListener("click", event =>
94 {
95 if (event.target.id == "notification-close")
96 notificationElement.classList.add("closing");
97 else if (event.target.id == "notification-optout" ||
98 event.target.id == "notification-hide")
99 {
100 if (event.target.id == "notification-optout")
101 Notification.toggleIgnoreCategory("*", true);
102
103 notificationElement.hidden = true;
104 notification.onClicked();
105 }
106 }, true);
107 }, false); 119 }, false);
OLDNEW
« no previous file with comments | « dependencies ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld