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

Issue 29533613: Issue 5593 - Return new value from prefs.toggle (Closed)

Created:
Sept. 1, 2017, 12:06 p.m. by Manish Jethani
Modified:
Sept. 5, 2017, 1:55 a.m.
Reviewers:
Thomas Greiner
Base URL:
https://hg.adblockplus.org/adblockplusui/
Visibility:
Public.

Description

Issue 5593 - Return new value from prefs.toggle

Patch Set 1 #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -3 lines) Patch
M messageResponder.js View 1 chunk +3 lines, -3 lines 4 comments Download

Messages

Total messages: 4
Manish Jethani
Sept. 1, 2017, 12:07 p.m. (2017-09-01 12:07:02 UTC) #1
Manish Jethani
Patch Set 1 We'll need this for the changes to the popup. When we toggle ...
Sept. 1, 2017, 12:08 p.m. (2017-09-01 12:08:38 UTC) #2
Thomas Greiner
LGTM https://codereview.adblockplus.org/29533613/diff/29533614/messageResponder.js File messageResponder.js (right): https://codereview.adblockplus.org/29533613/diff/29533614/messageResponder.js#newcode350 messageResponder.js:350: return NotificationStorage.toggleIgnoreCategory("*"); On 2017/09/01 12:08:38, Manish Jethani wrote: ...
Sept. 4, 2017, 10:44 a.m. (2017-09-04 10:44:19 UTC) #3
Manish Jethani
Sept. 4, 2017, 11:30 a.m. (2017-09-04 11:30:02 UTC) #4
https://codereview.adblockplus.org/29533613/diff/29533614/messageResponder.js
File messageResponder.js (right):

https://codereview.adblockplus.org/29533613/diff/29533614/messageResponder.js...
messageResponder.js:352: return Prefs[message.key] = !Prefs[message.key];
On 2017/09/04 10:44:19, Thomas Greiner wrote:
> I assume you're planning to use it in
> https://codereview.adblockplus.org/29532767/diff/29533671/stats.js, correct?
> Because I don't see the return value being used anywhere.

Yes, specifically line 140 here in the new code:

https://codereview.adblockplus.org/29532767/diff/29533671/stats.js#newcode140

Powered by Google App Engine
This is Rietveld