Issue 4917771620057088: Issue 1505 - "Large Scale Deployments" section on adblockplus.org (Closed)
|
Created:
Nov. 3, 2014, 11:12 a.m. by saroyanm Modified:
Nov. 5, 2014, 3 p.m. Reviewers:
Thomas Greiner CC:
Felix Dahlke Visibility:
Public. |
DescriptionTicket:
https://issues.adblockplus.org/ticket/1505
Note: the title of Deployments page is "Large Scale Deployments".
Patch Set 1 #
Total comments: 14
Patch Set 2 : #
Total comments: 2
Patch Set 3 : #MessagesTotal messages: 7
@Thomas can you please have a look when you will have time. Note: the title of Deployments page is "Large Scale Deployments".
http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... File deployments.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:11: It’s FREE. When copy-pasting something from a document please make sure you're not including characters like ’ „ or “. Instead use ' and ". http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:11: It’s FREE. It's inconsistent to have a "." at the end here but in none of the other bullet points. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> The link's href and content do not match. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> Is this line meant to break here? It might look so in the text document but it doesn't have one. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:15: <p>Already deployed Adblock Plus? How was the experience? What can we do better? <a href="mailto:info@eyeo.com">Let us know!</a></p> This should link to "mailto:deployment-support@adblockplus.org". http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:16: <p><a href="https://adblockplus.org/forum/viewtopic.php?f=21&t=24653&sid=db46979e2327f9f608b4a4945fe5dd6d">More about the project</a></p> No need to include the session ID in the link.
On 2014/11/03 11:16:57, saroyanm wrote: > Note: the title of Deployments page is "Large Scale Deployments". The text document says "Adblock Plus for Professionals" so if the title has changed please mention that in the issue description.
Result of sleeplessness :/ http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... File deployments.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:11: It’s FREE. On 2014/11/03 17:32:18, Thomas Greiner wrote: > When copy-pasting something from a document please make sure you're not > including characters like ’ „ or “. Instead use ' and ". Done. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:11: It’s FREE. On 2014/11/03 17:32:18, Thomas Greiner wrote: > It's inconsistent to have a "." at the end here but in none of the other bullet > points. Done. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> On 2014/11/03 17:32:18, Thomas Greiner wrote: > The link's href and content do not match. Oooopsss.. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> On 2014/11/03 17:32:18, Thomas Greiner wrote: > Is this line meant to break here? It might look so in the text document but it > doesn't have one. Yes I used break here, because it was breaking after "-" sign. So in the live example it will look like this: deployment- support@adblockplus.org http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:15: <p>Already deployed Adblock Plus? How was the experience? What can we do better? <a href="mailto:info@eyeo.com">Let us know!</a></p> On 2014/11/03 17:32:18, Thomas Greiner wrote: > This should link to "mailto:deployment-support@adblockplus.org". Oooopsss.. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:16: <p><a href="https://adblockplus.org/forum/viewtopic.php?f=21&t=24653&sid=db46979e2327f9f608b4a4945fe5dd6d">More about the project</a></p> On 2014/11/03 17:32:18, Thomas Greiner wrote: > No need to include the session ID in the link. ouch. http://codereview.adblockplus.org/4917771620057088/diff/5750085036015616/foot... File footer.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5750085036015616/foot... footer.html:29: <li><a href="en/deployments">Deployments</a></li> I think "deployments" is better for consistency.
http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... File deployments.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> On 2014/11/03 18:17:57, saroyanm wrote: > On 2014/11/03 17:32:18, Thomas Greiner wrote: > > Is this line meant to break here? It might look so in the text document but it > > doesn't have one. > > Yes I used break here, because it was breaking after "-" sign. > So in the live example it will look like this: > deployment- > mailto:support@adblockplus.org Remember that this site will be translated into other languages. Therefore how about specifying its style as "white-space: nowrap" instead? This would allow it to stay on the same line as long as there's space for the entire email address and otherwise move to a new line. http://codereview.adblockplus.org/4917771620057088/diff/5750085036015616/foot... File footer.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5750085036015616/foot... footer.html:29: <li><a href="en/deployments">Deployments</a></li> On 2014/11/03 18:17:57, saroyanm wrote: > I think "deployments" is better for consistency. I agree which is why I didn't comment on it. :)
Done. http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... File deployments.html (right): http://codereview.adblockplus.org/4917771620057088/diff/5629499534213120/depl... deployments.html:14: <p>We are happy to help you deploy it over your entire network! Please contact us at:<br/><a href="mailto:info@eyeo.com">deployment-support@adblockplus.org</a></p> On 2014/11/04 10:19:36, Thomas Greiner wrote: > On 2014/11/03 18:17:57, saroyanm wrote: > > On 2014/11/03 17:32:18, Thomas Greiner wrote: > > > Is this line meant to break here? It might look so in the text document but > it > > > doesn't have one. > > > > Yes I used break here, because it was breaking after "-" sign. > > So in the live example it will look like this: > > deployment- > > mailto:support@adblockplus.org > > Remember that this site will be translated into other languages. Therefore how > about specifying its style as "white-space: nowrap" instead? This would allow it > to stay on the same line as long as there's space for the entire email address > and otherwise move to a new line. Good point.
LGTM 
|
