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

Issue 29589609: Issue 5636 - Changed navbar contents (Closed)

Created:
Oct. 26, 2017, 11:53 a.m. by juliandoucette
Modified:
Jan. 23, 2018, 2:25 p.m.
Reviewers:
ire
Base URL:
https://bitbucket.org/adblockplus/adblockplus.org
Visibility:
Public.

Description

Issue 5636 - Changed navbar contents

Patch Set 1 #

Total comments: 2

Patch Set 2 : Changed order of about and contribute #

Patch Set 3 : Rebased, Changed documentation link, added globe icon, removed locale code #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -4 lines) Patch
M locales/en/menu.json View 1 chunk +3 lines, -0 lines 0 comments Download
M static/css/main.css View 1 2 1 chunk +15 lines, -0 lines 0 comments Download
A static/img/globe.png View 1 2 Binary file 0 comments Download
A static/img/globe.svg View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M templates/default.tmpl View 1 2 1 chunk +12 lines, -4 lines 0 comments Download

Messages

Total messages: 10
juliandoucette
Oct. 26, 2017, 11:54 a.m. (2017-10-26 11:54:02 UTC) #1
ire
One comment. https://codereview.adblockplus.org/29589609/diff/29589610/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29589609/diff/29589610/templates/default.tmpl#newcode96 templates/default.tmpl:96: {% for pagename in ["contribute", "about"] %} ...
Oct. 27, 2017, 12:47 p.m. (2017-10-27 12:47:27 UTC) #2
juliandoucette
@Jeen please find a question below. https://codereview.adblockplus.org/29589609/diff/29589610/templates/default.tmpl File templates/default.tmpl (right): https://codereview.adblockplus.org/29589609/diff/29589610/templates/default.tmpl#newcode96 templates/default.tmpl:96: {% for pagename ...
Oct. 28, 2017, 11:35 a.m. (2017-10-28 11:35:16 UTC) #3
jeen
On 2017/10/28 11:35:16, juliandoucette wrote: > @Jeen please find a question below. > > https://codereview.adblockplus.org/29589609/diff/29589610/templates/default.tmpl ...
Oct. 30, 2017, 10:29 a.m. (2017-10-30 10:29:46 UTC) #4
juliandoucette
On 2017/10/30 10:29:46, jeen wrote: > Yes the order should be: 'About' then 'Contribute' - ...
Oct. 30, 2017, 12:08 p.m. (2017-10-30 12:08:06 UTC) #5
ire
LGTM
Oct. 31, 2017, 7:48 a.m. (2017-10-31 07:48:29 UTC) #6
juliandoucette
@Jeen This is currently blocked by: https://bitbucket.org/adblockplus/spec/pull-requests/76/added-abp-website-specifications/diff#Lspec/adblockplus.org/abp-website.mdT61
Nov. 22, 2017, 3:44 p.m. (2017-11-22 15:44:26 UTC) #7
juliandoucette
@Jeen I'd also like to address https://bitbucket.org/adblockplus/spec/pull-requests/76/added-abp-website-specifications/diff#Lspec/adblockplus.org/abp-website.mdT66 too if it doesn't delay publishing too much. ...
Nov. 22, 2017, 3:46 p.m. (2017-11-22 15:46:01 UTC) #8
juliandoucette
On 2017/11/22 15:46:01, juliandoucette wrote: > @Jeen > > I'd also like to address > ...
Nov. 27, 2017, 5:34 p.m. (2017-11-27 17:34:11 UTC) #9
jeen
Nov. 28, 2017, 3:11 p.m. (2017-11-28 15:11:59 UTC) #10
On 2017/11/27 17:34:11, juliandoucette wrote:
> On 2017/11/22 15:46:01, juliandoucette wrote:
> > @Jeen
> > 
> > I'd also like to address
> >
>
https://bitbucket.org/adblockplus/spec/pull-requests/76/added-abp-website-spe...
> > too if it doesn't delay publishing too much.
> > 
> > (The suggestion about using the i18n icon)
> 
> The globe icon was decided on in the spec.
> 
> @Jeen The locale text didn't look right to me with the globe icon and the
locale
> code. As a result, I removed the locale codes in the latest PatchSet. I don't
> think that this code adds any not-stylistic value? Anyway, if you disagree,
> would you mind having a look at the [globe, text, code] combination and
letting
> me know if you think it's okay already or if you want to adjust it a bit?

I'm happy with removing the locale codes.

Powered by Google App Engine
This is Rietveld