updates.html - Issue 29592569: Issue 5943 - Implement Updates Page for Adblock Plus extension

Side by Side Diff: updates.html

Issue 29592569: Issue 5943 - Implement Updates Page for Adblock Plus extension (Closed) Base URL: https://hg.adblockplus.org/adblockplusui/

Patch Set: Addressed initial round of feedback Created Nov. 3, 2017, 10:01 a.m.

Left:
Right:

Use n/p to move between diff chunks; N/P to move between comments.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 <!--

	2 - This file is part of Adblock Plus <https://adblockplus.org/>,

	3 - Copyright (C) 2006-present eyeo GmbH

	4 -

	5 - Adblock Plus is free software: you can redistribute it and/or modify

	6 - it under the terms of the GNU General Public License version 3 as

	7 - published by the Free Software Foundation.

	8 -

	9 - Adblock Plus is distributed in the hope that it will be useful,

	10 - but WITHOUT ANY WARRANTY; without even the implied warranty of

	11 - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

	12 - GNU General Public License for more details.

	13 -

	14 - You should have received a copy of the GNU General Public License

	15 - along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.

	16 -->

	17 <!DOCTYPE html>

	18 <html>

	19 <head>

	20 <title class="i18n_firstRun_title"></title>
	Thomas Greiner 2017/11/03 17:40:01 Coding style: "Indentation: Two spaces per logic l Coding style: "Indentation: Two spaces per logic level" See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style... Thomas Greiner 2017/11/03 17:40:02 This class name refers to the wrong message ID. This class name refers to the wrong message ID. martin 2017/11/06 16:05:46 Done. Show quoted text On 2017/11/03 17:40:01, Thomas Greiner wrote: > Coding style: "Indentation: Two spaces per logic level" > > See > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style... Done. martin 2017/11/06 16:05:46 Done. Show quoted text On 2017/11/03 17:40:01, Thomas Greiner wrote: > Coding style: "Indentation: Two spaces per logic level" > > See > https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style... Done.
	21 <meta charset="utf-8" />

	22 <meta name="viewport" content="width=device-width, initial-scale=1" />
	Thomas Greiner 2017/11/03 17:40:02 I'm fine with adding this to have a more pleasant I'm fine with adding this to have a more pleasant experience on mobile devices although the design is a bit broken because the text "Adblock Plus has been updated!" is oddly placed on smaller screens. Just wanted to mention this in case you haven't been aware of this breakage in the design. martin 2017/11/06 16:05:50 This page would never be viewed from a mobile devi Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > I'm fine with adding this to have a more pleasant experience on mobile devices > although the design is a bit broken because the text "Adblock Plus has been > updated!" is oddly placed on smaller screens. Just wanted to mention this in > case you haven't been aware of this breakage in the design. This page would never be viewed from a mobile device, so I don't think optimising for mobile should be a priority, however I included a couple of CSS rules to fix the breakage anyways. Thomas Greiner 2017/11/14 12:25:39 The spec doesn't mention that this should only app Show quoted text On 2017/11/06 16:05:50, martin wrote: > On 2017/11/03 17:40:02, Thomas Greiner wrote: > > I'm fine with adding this to have a more pleasant experience on mobile devices > > although the design is a bit broken because the text "Adblock Plus has been > > updated!" is oddly placed on smaller screens. Just wanted to mention this in > > case you haven't been aware of this breakage in the design. > > This page would never be viewed from a mobile device, so I don't think > optimising for mobile should be a priority, however I included a couple of CSS > rules to fix the breakage anyways. The spec doesn't mention that this should only appear in Chrome but yeah, practically, it will never be seen except for Firefox Mobile users who haven't received the web extension update yet. So I agree, let's keep it as is.
	23 <link type="text/css" href="skin/common.css" rel="stylesheet" />
	Thomas Greiner 2017/11/03 17:40:02 Detail: None of the styles included in common.css Detail: None of the styles included in common.css are used for this page so let's not include it. martin 2017/11/06 16:05:46 Done. Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > Detail: None of the styles included in common.css are used for this page so > let's not include it. Done.
	24 <link type="text/css" href="skin/updates.css" rel="stylesheet"/>

	25 <script type="text/javascript" src="polyfill.js"></script>
	Thomas Greiner 2017/11/03 17:40:02 Detail: The "type" attribute has been made optiona Detail: The "type" attribute has been made optional in HTML5 so we don't need to specify it anymore. martin 2017/11/06 16:05:49 Done. Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > Detail: The "type" attribute has been made optional in HTML5 so we don't need to > specify it anymore. Done.
	26 <script type="text/javascript" src="ext/common.js"></script>

	27 <script type="text/javascript" src="ext/content.js"></script>

	28 <script type="text/javascript" src="common.js"></script>

	29 <script type="text/javascript" src="i18n.js"></script>

	30 </head>

	31

	32 <body>

	33

	34 <main>

	35 <div id="container">

	36

	37 <header class="column graphic-column">

	38 <div class="update-graphic-container">

	39 <div class="update-graphic-content">

	40 <img src="/skin/updates-page/abp-logo.svg" alt="adblock- plus-logo"/>
	Thomas Greiner 2017/11/03 17:40:02 Detail: Texts in "alt" attributes are just as visi Detail: Texts in "alt" attributes are just as visible to visually-impaired users as other texts which is why we should translate them. However, in this case it merely describes a decorative element so I'd suggest removing it here and for all other occurrences where this applies. martin 2017/11/06 16:05:49 Done. Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > Detail: Texts in "alt" attributes are just as visible to visually-impaired users > as other texts which is why we should translate them. However, in this case it > merely describes a decorative element so I'd suggest removing it here and for > all other occurrences where this applies. Done.
	41 <div class="version-details">

	42 <h2>Update Complete</h2>
	Thomas Greiner 2017/11/03 17:40:02 We mentioned it in today's meeting but just for co We mentioned it in today's meeting but just for completeness sake I wanted to note that we want to have the text on this page be translatable. Thomas Greiner 2017/11/03 17:40:03 Detail: I noticed that if this text is longer (i.e Detail: I noticed that if this text is longer (i.e. when it's translated to a different language) that it can slightly overflow to the right so we could simply add a margin to `.version-details` to avoid this issue. martin 2017/11/06 16:05:49 Done. Show quoted text On 2017/11/03 17:40:03, Thomas Greiner wrote: > Detail: I noticed that if this text is longer (i.e. when it's translated to a > different language) that it can slightly overflow to the right so we could > simply add a margin to `.version-details` to avoid this issue. Done. martin 2017/11/06 16:05:49 Done. Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > We mentioned it in today's meeting but just for completeness sake I wanted to > note that we want to have the text on this page be translatable. Done.
	43 <span>v1.13.4</span>
	Thomas Greiner 2017/11/03 17:40:01 Detail: The version number may end up being differ Detail: The version number may end up being different than what we define here so I think we got two options: Either we remove the version number or we set it dynamically the way we do it in the new options page (see https://hg.adblockplus.org/adblockplusui/file/tip/desktop-options.js#l878). martin 2017/11/06 16:05:46 I removed the version number <span> to not delay i Show quoted text On 2017/11/03 17:40:01, Thomas Greiner wrote: > Detail: The version number may end up being different than what we define here > so I think we got two options: Either we remove the version number or we set it > dynamically the way we do it in the new options page (see > https://hg.adblockplus.org/adblockplusui/file/tip/desktop-options.js#l878). I removed the version number <span> to not delay implementation unnecessarily.
	44 </div>

	45

	46 </div>

	47 </header>

	48

	49 <section class="column content-column">

	50

	51 <div id="content">

	52

	53 <header>

	54 <h1>Adblock Plus has been updated!</h1>

	55 <p>What's new?</p>

	56 </header>

	57

	58 <article class="feature-entry">

	59 <img class="feature-icon" src="/skin/updates-page/ic on-rocket.svg" alt="rocket-icon"/>

	60 <p>All new Options page - revamped and restyled - so it’s easier to customize Adblock Plus just like you want it. <a href="#">Check it out for yourself</a>.</p>
	Thomas Greiner 2017/11/03 17:40:01 Detail: We try to normalize strings, for instance Detail: We try to normalize strings, for instance by replacing various variants of apostrophe's such as "’" with "'", to avoid running into issues with translations, screen readers, etc. Thomas Greiner 2017/11/03 17:40:02 This link and the app store buttons below are miss This link and the app store buttons below are missing the URL. This one I don't see in the spec either. Note that we use documentation links (see https://bitbucket.org/adblockplus/spec/src/8e6b8ee2b7a1484344688baa405e11ab42...) so in case those links are not yet specified in the spec, we should add them and create the necessary redirect for them on our servers. The reason for that is that links may break over time and due to the fact that people can install this version of the extension at any point in time in the future. Therefore we link to adblockplus.org instead from where we redirect the user to the appropriate URL. martin 2017/11/06 16:05:46 Done. Show quoted text On 2017/11/03 17:40:01, Thomas Greiner wrote: > Detail: We try to normalize strings, for instance by replacing various variants > of apostrophe's such as "’" with "'", to avoid running into issues with > translations, screen readers, etc. Done.
	61 </article>

	62

	63 <article class="feature-entry">

	64 <img class="feature-icon" src="/skin/updates-page/ic on-thumbs-up.svg" alt="thumbs-up-icon"/>

	65 <p>Block ads on Facebook again! New blocking tech al lows for fast fixes in case <strike>the Empire</strike> Facebook strikes back.</ p>
	Thomas Greiner 2017/11/03 17:40:02 The `<strike>` element has been deprecated in HTML The `<strike>` element has been deprecated in HTML4 according to https://developer.mozilla.org/en-US/docs/Web/HTML/Element/strike. Instead, it suggests to use either `<del>` or `<s>` but we only support `<strong>` and `<a>` in translatable strings. Therefore I'd recommend using `<strong>` and styling it via CSS. martin 2017/11/06 16:05:46 Done. Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > The `<strike>` element has been deprecated in HTML4 according to > https://developer.mozilla.org/en-US/docs/Web/HTML/Element/strike. Instead, it > suggests to use either `<del>` or `<s>` but we only support `<strong>` and `<a>` > in translatable strings. > > Therefore I'd recommend using `<strong>` and styling it via CSS. Done.
	66 </article>

	67

	68 <article class="custom-feature-entry">

	69 <h2>But what about your phone?</h2>

	70 <div class="feature-entry">

	71 <img class="feature-icon" src="/skin/updates-pag e/icon-mobile.svg" alt="thumbs-up-icon"/>

	72 <p>If you like taking control on desktop, you ca n block ads and improve your privacy and security on mobile, too. Get the Adbloc k Browser App below.</p>

	73 </div>

	74 <div class="store-buttons">

	75 <a href="" class="store-button applestore-button ">

	76 <img src="skin/updates-page/appstore-bg.svg" alt="apple store button">

	77 </a>

	78 <a href="" class="store-button googleplay-button ">

	79 <img src="skin/updates-page/googleplay-bg.sv g" alt="google play store button">
	Thomas Greiner 2017/11/03 17:40:01 Are we allowed to use those images? Because accord Are we allowed to use those images? Because according to https://play.google.com/intl/en_us/badges/ we're not allowed to alter the badges and presumably, Apple's guidelines have similar requirements. Thomas Greiner 2017/11/03 17:40:02 These images contain text which we means that we c These images contain text which we means that we cannot translate it. Therefore we need to specify the text in the HTML and either style the button using HTML and CSS or make changes to the design. martin 2017/11/06 16:05:49 We could potentially do that, but it'll slow us do Show quoted text On 2017/11/03 17:40:02, Thomas Greiner wrote: > These images contain text which we means that we cannot translate it. Therefore > we need to specify the text in the HTML and either style the button using HTML > and CSS or make changes to the design. We could potentially do that, but it'll slow us down. I truly believe the buttons speak for themselves and there's no need translating the text inside. Not to mention that in order to adhere to the guidelines 100% the fonts inside the buttons must be SanFrancisco and Roboto respectively. martin 2017/11/06 16:05:50 Apple store button is in accordance with their gui Show quoted text On 2017/11/03 17:40:01, Thomas Greiner wrote: > Are we allowed to use those images? Because according to > https://play.google.com/intl/en_us/badges/ we're not allowed to alter the badges > and presumably, Apple's guidelines have similar requirements. Apple store button is in accordance with their guidelines. I exchanged the monochrome graphic with a coloured one for the Google Play button and changed the text. It now adheres to the guidelines. Thomas Greiner 2017/11/14 12:25:40 Great, thanks! Show quoted text On 2017/11/06 16:05:50, martin wrote: > On 2017/11/03 17:40:01, Thomas Greiner wrote: > > Are we allowed to use those images? Because according to > > https://play.google.com/intl/en_us/badges/ we're not allowed to alter the > badges > > and presumably, Apple's guidelines have similar requirements. > > Apple store button is in accordance with their guidelines. I exchanged the > monochrome graphic with a coloured one for the Google Play button and changed > the text. It now adheres to the guidelines. Great, thanks!
	80 </a>

	81 </div>

	82 </article>

	83

	84 </div>

	85

	86 </section>

	87

	88 </div>

	89

	90 </main>

	91

	92 </body>

	93

	94 </html>

	95

OLD	NEW

« skin/updates-page/googleplay-bg.svg ('K') | « skin/updates-page/icon-thumbs-up.svg ('k') | no next file » | no next file with comments »