Issue 6063199216467968: Issue 2642 - Moved notifications code to separate module

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 6063199216467968: Issue 2642 - Moved notifications code to separate module (Closed)

Created:
June 4, 2015, 9:31 p.m. by Sebastian Noack

Modified:
June 5, 2015, 4:36 p.m.

Reviewers:
Thomas Greiner

CC:
Wladimir Palant, kzar

Visibility:
Public.

Description

Issue 2642 - Moved notifications code to separate module

Patch Set 1 #

Total comments: 1

Patch Set 2 : Use let instead var keyword #

Patch Set 3 : Cleanup comparisons for consistency #

Patch Set 4 : Turned showNotification(notification) into showNextNotification([url]) to remove code duplication #

Patch Set 5 : Moved intilization code into module #

Total comments: 2

Patch Set 6 : Added @module tag #

Patch Set 7 : Fixed typo in comment #

Created: June 5, 2015, 2:06 p.m.

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+236 lines, -211 lines)			Patch
M	background.js	View	1 2 3 4	6 chunks	+3 lines, -203 lines	0 comments	Download
A	lib/notificationHelper.js	View	1 2 3 4 5 6	1 chunk	+229 lines, -0 lines	0 comments	Download
M	metadata.common	View		1 chunk	+1 line, -0 lines	0 comments	Download
M	webrequest.js	View	1 2 3	3 chunks	+3 lines, -8 lines	0 comments	Download

Messages

Total messages: 4

Expand All Messages | Collapse All Messages

Sebastian Noack

For easier review, I split up this change into multiple patch sets. Note that the ...

June 4, 2015, 9:38 p.m. (2015-06-04 21:38:15 UTC) #1

Thomas Greiner

LGTM, just a comment regarding a spelling mistake. Note that there's also one in the ...

June 5, 2015, 1:50 p.m. (2015-06-05 13:50:52 UTC) #3

Sebastian Noack

June 5, 2015, 2:06 p.m. (2015-06-05 14:06:12 UTC) #4

http://codereview.adblockplus.org/6063199216467968/diff/5676830073815040/lib/...
File lib/notificationHelper.js (right):

http://codereview.adblockplus.org/6063199216467968/diff/5676830073815040/lib/...
lib/notificationHelper.js:131: * Initilizes the notification system.
On 2015/06/05 13:50:52, Thomas Greiner wrote:
> Replace "Initilizes" with "Initializes"

Done.

Expand All Messages | Collapse All Messages