Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(9)

Issue 29730611: Issue 6511 - Use messaging to register popup notification clicks (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 6 months ago by kzar
Modified:
1 year, 5 months ago
CC:
Thomas Greiner
Visibility:
Public.

Description

Issue 6511 - Use messaging to register popup notification clicks Depends on https://codereview.adblockplus.org/29730608/

Patch Set 1 #

Total comments: 6

Patch Set 2 : Updated dependency #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -2 lines) Patch
M dependencies View 1 1 chunk +1 line, -1 line 0 comments Download
M lib/notificationHelper.js View 1 1 chunk +9 lines, -0 lines 1 comment Download
M popup.js View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 11
kzar
Patch Set 1
1 year, 6 months ago (2018-03-22 15:06:39 UTC) #1
saroyanm
https://codereview.adblockplus.org/29730611/diff/29730612/dependencies File dependencies (right): https://codereview.adblockplus.org/29730611/diff/29730612/dependencies#newcode5 dependencies:5: adblockplusui = adblockplusui git:6512-notification-close I assume this suppose to ...
1 year, 6 months ago (2018-03-22 17:30:47 UTC) #2
kzar
https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome#newcode115 metadata.chrome:115: desktop-options.js = adblockplusui/js/desktop-options.js On 2018/03/22 17:30:47, saroyanm wrote: > ...
1 year, 6 months ago (2018-03-22 17:36:50 UTC) #3
saroyanm
https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome#newcode115 metadata.chrome:115: desktop-options.js = adblockplusui/js/desktop-options.js On 2018/03/22 17:36:50, kzar wrote: > ...
1 year, 6 months ago (2018-03-22 18:03:19 UTC) #4
Sebastian Noack
Looks good to me, once the rebased on the dependency update.
1 year, 6 months ago (2018-03-22 18:06:26 UTC) #5
a.giammarchi
I was going through reviews and I've noticed a stopper in here. https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome File metadata.chrome ...
1 year, 5 months ago (2018-03-23 14:24:56 UTC) #6
kzar
https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29730611/diff/29730612/metadata.chrome#newcode115 metadata.chrome:115: desktop-options.js = adblockplusui/js/desktop-options.js On 2018/03/23 14:24:56, a.giammarchi wrote: > ...
1 year, 5 months ago (2018-03-23 14:43:12 UTC) #7
a.giammarchi
On 2018/03/23 14:43:12, kzar wrote: > As I mentioned I will update this codereview once ...
1 year, 5 months ago (2018-03-23 14:59:07 UTC) #8
kzar
On 2018/03/23 14:59:07, a.giammarchi wrote: > If you include that file, nothing would work in ...
1 year, 5 months ago (2018-03-23 15:15:27 UTC) #9
kzar
Patch Set 2 : Updated dependency https://codereview.adblockplus.org/29730611/diff/29731614/lib/notificationHelper.js File lib/notificationHelper.js (right): https://codereview.adblockplus.org/29730611/diff/29731614/lib/notificationHelper.js#newcode129 lib/notificationHelper.js:129: showOptions((page, port) => ...
1 year, 5 months ago (2018-03-23 16:41:52 UTC) #10
Sebastian Noack
1 year, 5 months ago (2018-03-23 22:44:03 UTC) #11
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5