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

Issue 29592569: Issue 5943 - Implement Updates Page for Adblock Plus extension (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 months, 3 weeks ago by martin
Modified:
9 months, 1 week ago
CC:
juliandoucette
Base URL:
https://hg.adblockplus.org/adblockplusui/
Visibility:
Public.

Description

Issue 5943 - Implement Updates Page for Adblock Plus extension

Patch Set 1 #

Total comments: 26

Patch Set 2 : Addressed initial round of feedback #

Total comments: 65

Patch Set 3 : Addressed second round of feedback #

Patch Set 4 : Addressed second round of feedback #

Total comments: 32

Patch Set 5 : Addressed third round of feedback #

Total comments: 14

Patch Set 6 : Addressed fourth round of feedback #

Total comments: 15

Patch Set 7 : Addressed fifth round of feedback #

Total comments: 21

Patch Set 8 : Addressed sixth round of feedback #

Total comments: 14

Patch Set 9 : Addressed seventh round of feedback #

Total comments: 1

Patch Set 10 : Addressed seventh round of feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+463 lines, -0 lines) Patch
A locale/en_US/updates.json View 1 2 3 4 5 6 7 8 1 chunk +23 lines, -0 lines 0 comments Download
A skin/updates.css View 1 2 3 4 5 6 7 8 9 1 chunk +247 lines, -0 lines 0 comments Download
A skin/updates/appstore-bg.svg View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A skin/updates/base-graphic.svg View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A skin/updates/googleplay-bg.svg View 1 2 3 4 5 1 chunk +41 lines, -0 lines 0 comments Download
A skin/updates/icon-mobile.svg View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A skin/updates/icon-rocket.svg View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A skin/updates/icon-thumbs-up.svg View 1 2 1 chunk +1 line, -0 lines 0 comments Download
A updates.html View 1 2 3 4 5 6 7 8 1 chunk +97 lines, -0 lines 0 comments Download
A updates.js View 1 2 3 4 5 6 7 8 1 chunk +50 lines, -0 lines 0 comments Download

Messages

Total messages: 26
martin
11 months, 3 weeks ago (2017-10-30 10:37:35 UTC) #1
martin
Hello there, Submitted an initial implementation of the updates page. Will try and address any ...
11 months, 3 weeks ago (2017-10-30 10:39:09 UTC) #2
juliandoucette
Is this supposed to work on mobile? https://codereview.adblockplus.org/29592569/diff/29592570/skin/updates.css File skin/updates.css (right): https://codereview.adblockplus.org/29592569/diff/29592570/skin/updates.css#newcode11 skin/updates.css:11: @font-face NIT: ...
11 months, 3 weeks ago (2017-10-30 15:30:11 UTC) #3
juliandoucette
(Note: Above are my first impressions. I stopped reviewing after updates.html and the first part ...
11 months, 3 weeks ago (2017-10-30 15:33:46 UTC) #4
martin
Hey all. I (hopefully) addressed the first feedback round. @juliandoucette: from what I know from ...
11 months, 2 weeks ago (2017-11-03 10:02:15 UTC) #5
juliandoucette
Quick reply. Thank Martin! https://codereview.adblockplus.org/29592569/diff/29592570/updates.html File updates.html (right): https://codereview.adblockplus.org/29592569/diff/29592570/updates.html#newcode3 updates.html:3: - This file is part ...
11 months, 2 weeks ago (2017-11-03 11:06:06 UTC) #6
Thomas Greiner
The main points I mentioned are around translations, links and the app store badges. Apart ...
11 months, 2 weeks ago (2017-11-03 17:40:03 UTC) #7
martin
Hey all, I submitted a second review, addressing Thomas's feedback. Let me know if I ...
11 months, 2 weeks ago (2017-11-06 16:05:50 UTC) #8
Thomas Greiner
Make sure you're always on the latest revision and avoid reverting changes made by others ...
11 months, 2 weeks ago (2017-11-09 13:57:31 UTC) #9
Thomas Greiner
Mostly details to address but we should get the links and store buttons resolved. https://codereview.adblockplus.org/29592569/diff/29596582/skin/updates-page/abp-logo.svg ...
11 months, 1 week ago (2017-11-14 12:25:45 UTC) #10
martin
Pushed a new review, addressing (almost all) comments from the previous one. https://codereview.adblockplus.org/29592569/diff/29596582/skin/updates-page/abp-logo.svg File skin/updates-page/abp-logo.svg ...
11 months ago (2017-11-20 11:47:36 UTC) #11
Thomas Greiner
There are mostly JavaScript related issues left. https://codereview.adblockplus.org/29592569/diff/29596582/skin/updates.css File skin/updates.css (right): https://codereview.adblockplus.org/29592569/diff/29596582/skin/updates.css#newcode131 skin/updates.css:131: left: 100px; ...
11 months ago (2017-11-20 17:06:08 UTC) #12
martin
Sending a new patch your way. Let me know if I missed something. https://codereview.adblockplus.org/29592569/diff/29605738/skin/updates.css File ...
10 months, 3 weeks ago (2017-11-26 17:08:41 UTC) #13
Thomas Greiner
https://codereview.adblockplus.org/29592569/diff/29612630/updates.html File updates.html (right): https://codereview.adblockplus.org/29592569/diff/29612630/updates.html#newcode75 updates.html:75: <a href="https://eyeo.to/adblockbrowser/ios/update-page" class="store-button applestore-button"> On 2017/11/26 17:08:39, martin wrote: ...
10 months, 3 weeks ago (2017-11-27 16:15:52 UTC) #14
martin
Submitted a new review. https://codereview.adblockplus.org/29592569/diff/29612630/updates.html File updates.html (right): https://codereview.adblockplus.org/29592569/diff/29612630/updates.html#newcode75 updates.html:75: <a href="https://eyeo.to/adblockbrowser/ios/update-page" class="store-button applestore-button"> On ...
10 months, 2 weeks ago (2017-12-05 14:37:33 UTC) #15
Thomas Greiner
https://codereview.adblockplus.org/29592569/diff/29619567/skin/updates.css File skin/updates.css (right): https://codereview.adblockplus.org/29592569/diff/29619567/skin/updates.css#newcode78 skin/updates.css:78: cursor: pointer; On 2017/12/05 14:37:31, martin wrote: > On ...
10 months, 1 week ago (2017-12-11 12:11:12 UTC) #16
martin
https://codereview.adblockplus.org/29592569/diff/29630703/updates.html File updates.html (right): https://codereview.adblockplus.org/29592569/diff/29630703/updates.html#newcode77 updates.html:77: <a id="adblock_browser_ios_store" class="store-button applestore-button" target="_blank"> On 2017/12/11 12:11:10, Thomas ...
10 months, 1 week ago (2017-12-14 11:25:17 UTC) #17
Thomas Greiner
https://codereview.adblockplus.org/29592569/diff/29630703/updates.html File updates.html (right): https://codereview.adblockplus.org/29592569/diff/29630703/updates.html#newcode77 updates.html:77: <a id="adblock_browser_ios_store" class="store-button applestore-button" target="_blank"> On 2017/12/14 11:25:16, martin ...
10 months, 1 week ago (2017-12-14 13:36:28 UTC) #18
martin
Published a new patch-set. I might have missed something - do let me know if ...
10 months ago (2017-12-19 12:59:53 UTC) #19
juliandoucette
Moving myself to CC.
9 months, 2 weeks ago (2018-01-04 15:27:09 UTC) #20
saroyanm
Were checking the difference between requested translations and strings here, seems like some of the ...
9 months, 2 weeks ago (2018-01-05 14:01:50 UTC) #21
saroyanm
https://codereview.adblockplus.org/29592569/diff/29644588/locale/en_US/updates.json File locale/en_US/updates.json (right): https://codereview.adblockplus.org/29592569/diff/29644588/locale/en_US/updates.json#newcode3 locale/en_US/updates.json:3: "message": "Update Complete" On 2018/01/05 14:01:47, saroyanm wrote: > ...
9 months, 2 weeks ago (2018-01-05 14:09:11 UTC) #22
Thomas Greiner
The Adblock Browser website link doesn't work yet but other than that there's only a ...
9 months, 1 week ago (2018-01-10 12:28:25 UTC) #23
martin
Addressed the latest feedback and submitted a new patch-set. https://codereview.adblockplus.org/29592569/diff/29630703/skin/updates.css File skin/updates.css (right): https://codereview.adblockplus.org/29592569/diff/29630703/skin/updates.css#newcode54 skin/updates.css:54: ...
9 months, 1 week ago (2018-01-12 11:15:47 UTC) #24
Thomas Greiner
LGTM with the coding style comment addressed https://codereview.adblockplus.org/29592569/diff/29664579/skin/updates.css File skin/updates.css (right): https://codereview.adblockplus.org/29592569/diff/29664579/skin/updates.css#newcode49 skin/updates.css:49: #graphic-column, .graphic-column ...
9 months, 1 week ago (2018-01-12 11:31:45 UTC) #25
Thomas Greiner
9 months, 1 week ago (2018-01-12 14:58:19 UTC) #26
One more thing: I noticed that the feature icons are larger than what's shown in
the spec and some other dimensions seem to be off too. However, no need to block
this review because of those small details so feel free to adapt the styles in a
follow-up change.
Sign in to reply to this message.

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