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

Issue 29801613: Issue 6321 - Move the popup under UI (Closed)

Created:
June 7, 2018, 1:22 p.m. by a.giammarchi
Modified:
Nov. 7, 2018, 2:43 p.m.
Visibility:
Public.

Description

Already landed.

Patch Set 1 #

Patch Set 2 : removed popup related translations #

Patch Set 3 : #

Total comments: 4

Patch Set 4 : applied required changes #

Patch Set 5 : importing strings from popup.json #

Patch Set 6 : added missing files #

Total comments: 1

Patch Set 7 : removed popup.png #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -840 lines) Patch
M _locales/en_US/messages.json View 1 3 chunks +0 lines, -56 lines 0 comments Download
R icons/detailed/abp-64.png View 1 2 3 Binary file 0 comments Download
M metadata.chrome View 1 2 3 4 5 6 4 chunks +15 lines, -1 line 0 comments Download
R popup.html View 1 chunk +0 lines, -105 lines 0 comments Download
R popup.js View 1 chunk +0 lines, -341 lines 0 comments Download
R skin/popup.css View 1 chunk +0 lines, -337 lines 0 comments Download
R skin/popup.png View Binary file 0 comments Download

Messages

Total messages: 15
a.giammarchi
I have already tested this locally, after moving all files inside the adblockplusui folder (and ...
June 7, 2018, 1:24 p.m. (2018-06-07 13:24:07 UTC) #1
Sebastian Noack
We probably should also move the translations used by the popup, as well as icons/detailed/abp-64.png, ...
June 7, 2018, 3:21 p.m. (2018-06-07 15:21:27 UTC) #2
a.giammarchi
On 2018/06/07 15:21:27, Sebastian Noack wrote: > We probably should also move the translations used ...
June 7, 2018, 3:33 p.m. (2018-06-07 15:33:27 UTC) #3
Sebastian Noack
On 2018/06/07 15:33:27, a.giammarchi wrote: > You mean to re-map all icons to adblockplusui/icons? > ...
June 7, 2018, 4:17 p.m. (2018-06-07 16:17:58 UTC) #4
a.giammarchi
I have done all popup related updates and I don't think we need to dig ...
June 8, 2018, 1:14 p.m. (2018-06-08 13:14:28 UTC) #5
Thomas Greiner
https://codereview.adblockplus.org/29801613/diff/29802559/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29801613/diff/29802559/metadata.chrome#newcode65 metadata.chrome:65: icons/detailed/abp-64.png = adblockplusui/skin/abp-64.png Detail: I don't see this file ...
June 8, 2018, 1:33 p.m. (2018-06-08 13:33:23 UTC) #6
a.giammarchi
dropped file and useless overrides in the geko meta file. https://codereview.adblockplus.org/29801613/diff/29802559/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29801613/diff/29802559/metadata.chrome#newcode65 ...
June 8, 2018, 1:44 p.m. (2018-06-08 13:44:23 UTC) #7
Thomas Greiner
LGTM Note that it's up to Sebastian to decide when and where this change should ...
June 11, 2018, 11:34 a.m. (2018-06-11 11:34:41 UTC) #8
a.giammarchi
On 2018/06/11 11:34:41, Thomas Greiner wrote: > LGTM > Note that it's up to Sebastian ...
June 11, 2018, 11:39 a.m. (2018-06-11 11:39:57 UTC) #9
a.giammarchi
FYI it looks like these strings are all duplicated in the popup.json within the adbplusui ...
June 19, 2018, 3 p.m. (2018-06-19 15:00:00 UTC) #10
a.giammarchi
actually, never mind. I think Thomas spotted the issue, and it was me duplicating the ...
June 19, 2018, 3:50 p.m. (2018-06-19 15:50:21 UTC) #11
Thomas Greiner
On 2018/06/19 15:50:21, a.giammarchi wrote: > Yet I think I need to add to the ...
June 19, 2018, 4:20 p.m. (2018-06-19 16:20:20 UTC) #12
a.giammarchi
On 2018/06/19 16:20:20, Thomas Greiner wrote: > On 2018/06/19 15:50:21, a.giammarchi wrote: > > Yet ...
June 19, 2018, 5:03 p.m. (2018-06-19 17:03:03 UTC) #13
Thomas Greiner
https://codereview.adblockplus.org/29801613/diff/29817555/metadata.chrome File metadata.chrome (right): https://codereview.adblockplus.org/29801613/diff/29817555/metadata.chrome#newcode144 metadata.chrome:144: skin/popup.png = adblockplusui/skin/popup.png We don't seem to be using ...
July 13, 2018, 10:22 a.m. (2018-07-13 10:22:17 UTC) #14
a.giammarchi
July 17, 2018, 4:22 p.m. (2018-07-17 16:22:56 UTC) #15
On 2018/07/13 10:22:17, Thomas Greiner wrote:
> https://codereview.adblockplus.org/29801613/diff/29817555/metadata.chrome
> File metadata.chrome (right):
> 
>
https://codereview.adblockplus.org/29801613/diff/29817555/metadata.chrome#new...
> metadata.chrome:144: skin/popup.png = adblockplusui/skin/popup.png
> We don't seem to be using this sprite image anymore in the new design so
there's
> nothing to import here.

Done.

Powered by Google App Engine
This is Rietveld