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

Issue 29948561: Issue 7093 - Only animate icon for global notifications (Closed)

Created:
Nov. 20, 2018, 5:48 p.m. by Thomas Greiner
Modified:
Nov. 22, 2018, 12:03 p.m.
Reviewers:
kzar
CC:
Sebastian Noack
Visibility:
Public.

Description

Issue 7093 - Only animate icon for global notifications

Patch Set 1 #

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

Messages

Total messages: 2
kzar
LGTM https://codereview.adblockplus.org/29948561/diff/29948562/lib/notificationHelper.js File lib/notificationHelper.js (right): https://codereview.adblockplus.org/29948561/diff/29948562/lib/notificationHelper.js#newcode43 lib/notificationHelper.js:43: let animateIcon = !(activeNotification.urlFilters instanceof Array) && Nit: ...
Nov. 20, 2018, 8:20 p.m. (2018-11-20 20:20:42 UTC) #1
Thomas Greiner
Nov. 21, 2018, 11:55 a.m. (2018-11-21 11:55:58 UTC) #2
https://codereview.adblockplus.org/29948561/diff/29948562/lib/notificationHel...
File lib/notificationHelper.js (right):

https://codereview.adblockplus.org/29948561/diff/29948562/lib/notificationHel...
lib/notificationHelper.js:43: let animateIcon = !(activeNotification.urlFilters
instanceof Array) &&
On 2018/11/20 20:20:42, kzar wrote:
> Nit: I figure we could just check that `urlFilters` is truthy instead of an
> Array instance, but I don't insist.

You're probably right. Given that this data is coming from the server, I'm not
sure I trust it enough with getting the format right though. :)

Powered by Google App Engine
This is Rietveld