Issue 29454619: Issue 4362 - Update and improve text on "Learn More" tab on adblockplus.org and rearrange links in … (Closed)
|
Created:
June 2, 2017, 12:06 p.m. by ire Modified:
Aug. 9, 2017, 8:52 a.m. CC:
Lisa, jeen Base URL:
https://hg.adblockplus.org/web.adblockplus.org Visibility:
Public. |
DescriptionIssue 4362 - Update and improve text on "Learn More" tab on adblockplus.org and rearrange links in alphabetical order
Patch Set 1 #Patch Set 2 : Remove inclues, place IDs before classes #MessagesTotal messages: 10
Some notes: I moved the content for each "Learn More" tab to a separate file for better readability. For some strings, I created a a new translation ID, but for others I left them the same. I did this based on if the content was significantly different enough, or if it was a minor re-wording. I wasn't sure if a new ID is required each time a change is made, so let me know if I should change all/more of them.
> For some strings, I created a a new translation ID, but for others I left them
the same. I did this based on if the content was significantly different enough,
or if it was a minor re-wording. I wasn't sure if a new ID is required each time
a change is made, so let me know if I should change all/more of them.
It depends on the subject. e.g. Don't change the ID if a string is completely
rewritten on the same subject. That being said, I think this makes little
difference to our translators if the string is completely changed and the ID is
changed. Especially if the old ID came from our old system ("s555").
Reminder: Fixing NITs is optional; but preferred. It often comes down to who has
a stronger opinion :D .
https://codereview.adblockplus.org/29454619/diff/29455555/includes/index.tmpl
File includes/index.tmpl (right):
https://codereview.adblockplus.org/29454619/diff/29455555/includes/index.tmpl...
includes/index.tmpl:199:
NIT: Unnecessary whitespace.
https://codereview.adblockplus.org/29454619/diff/29455555/includes/index.tmpl...
includes/index.tmpl:227: <? include learn-more/chrome ?>
NIT: We usually don't externalize things like this unless they appear on
multiple pages.
https://codereview.adblockplus.org/29454619/diff/29455555/includes/learn-more...
File includes/learn-more/chrome.tmpl (right):
https://codereview.adblockplus.org/29454619/diff/29455555/includes/learn-more...
includes/learn-more/chrome.tmpl:1: <div class="more-tab" id="tab-chrome">
NIT: We usually put id first
-- This isn't specified anywhere :/
https://codereview.adblockplus.org/29454619/diff/29455555/includes/learn-more...
includes/learn-more/chrome.tmpl:1: <div class="more-tab" id="tab-chrome">
NIT: We prefer HTML (.html) > Markdown (.md) > Template/Jinja2 (.tmpl). There is
no reason to use template format unless there is template logic (conditionals,
loops, macros, etc).
-- This isn't specified anywhere :/
I had an issue while uploading and somehow lost the inline comments :/ .
Replying below:
> > For some strings, I created a a new translation ID, but for others I left
them
> the same. I did this based on if the content was significantly different
enough,
> or if it was a minor re-wording. I wasn't sure if a new ID is required each
time
> a change is made, so let me know if I should change all/more of them.
>
> It depends on the subject. e.g. Don't change the ID if a string is completely
> rewritten on the same subject. That being said, I think this makes little
> difference to our translators if the string is completely changed and the ID
is
> changed. Especially if the old ID came from our old system ("s555").
Acknowledged.
> Reminder: Fixing NITs is optional; but preferred. It often comes down to who
has
> a stronger opinion :D .
Acknowledged. For the most part I'm still trying to learn how you do things
here, so not challenging most NITs unless I feel particularly strongly about it.
>
https://codereview.adblockplus.org/29454619/diff/29455555/includes/index.tmpl...
> includes/index.tmpl:199:
> NIT: Unnecessary whitespace.
Done.
>
https://codereview.adblockplus.org/29454619/diff/29455555/includes/index.tmpl...
> includes/index.tmpl:227: <? include learn-more/chrome ?>
> NIT: We usually don't externalize things like this unless they appear on
multiple pages.
Done and noted.
>
https://codereview.adblockplus.org/29454619/diff/29455555/includes/learn-more...
> includes/learn-more/chrome.tmpl:1: <div class="more-tab" id="tab-chrome">
> NIT: We usually put id first
> -- This isn't specified anywhere :/
Done and noted.
>
https://codereview.adblockplus.org/29454619/diff/29455555/includes/learn-more...
> includes/learn-more/chrome.tmpl:1: <div class="more-tab" id="tab-chrome">
> NIT: We prefer HTML (.html) > Markdown (.md) > Template/Jinja2 (.tmpl). There
is no reason to use template format unless there is template logic
(conditionals, loops, macros, etc).
> -- This isn't specified anywhere :/
Acknowledged.
> I had an issue while uploading and somehow lost the inline comments :/ . Did you delete and re-upload the first patchset? If not, it may be a bug, and we may want to pass this on to dev-ops.
LGTM - Except: @Tamara we have introduced a couple new string IDs here that we didn't need to. Do you think we should change them back so that we retain the old strings in languages that you do not translate?
@Tamara: Any update on this? On 2017/06/16 17:59:20, juliandoucette wrote: > @Tamara we have introduced a couple new string IDs here that we didn't need to. > Do you think we should change them back so that we retain the old strings in > languages that you do not translate?
On 2017/06/28 09:57:10, ire wrote: > @Tamara: Any update on this? > > On 2017/06/16 17:59:20, juliandoucette wrote: > > @Tamara we have introduced a couple new string IDs here that we didn't need > to. > > Do you think we should change them back so that we retain the old strings in > > languages that you do not translate? I am really sorry for the delay, busy at the moment with different releases. I will try to give you an answer as soon as next week if that's ok.
On 2017/06/28 13:29:13, tamara wrote: > I am really sorry for the delay, busy at the moment with different releases. I > will try to give you an answer as soon as next week if that's ok. Yes that's fine. Thank you.
+Jeen +Lisa RE: https://issues.adblockplus.org/ticket/4362#comment:17 Can/Should we push this change before we update the homepage this/next month? - It could slightly improve our searchability - We can wait for translations or push the change for English only (I know that you answered this question in the comment that I linked. I am asking again because I think that I am or may be providing new relevant information above.) 
|
