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 third round of feedback Created Nov. 20, 2017, 11:44 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 <!DOCTYPE html>

	2 <!--

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

	4 - Copyright (C) 2006-present eyeo GmbH

	5 -

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

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

	8 - published by the Free Software Foundation.

	9 -

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

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

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

	13 - GNU General Public License for more details.

	14 -

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

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

	17 -->

	18

	19 <html>

	20 <head>

	21 <title class="i18n_updates_status"></title>

	22 <meta charset="utf-8" />

	23 <meta name="viewport" content="width=device-width, initial-scale=1" />

	24 <link href="skin/updates.css" rel="stylesheet"/>

	25 <script src="polyfill.js"></script>

	26 <script src="ext/common.js"></script>

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

	28 <script src="common.js"></script>

	29 <script src="i18n.js"></script>

	30 <script src="updates.js"></script>
	Thomas Greiner 2017/11/20 17:06:07 This file is not included in the review so please This file is not included in the review so please make sure to add it. martin 2017/11/26 17:08:40 Done. Show quoted text On 2017/11/20 17:06:07, Thomas Greiner wrote: > This file is not included in the review so please make sure to add it. Done.
	31 </head>

	32

	33 <body>

	34

	35 <main>

	36 <div id="container">

	37

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

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

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

	41 <img src="/skin/abp-logo.svg"/>

	42 <div class="version-details">

	43 <h2 class="i18n_updates_status"></h2>

	44 </div>

	45

	46 </div>

	47 </header>

	48

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

	50

	51 <div id="content">

	52

	53 <header>

	54 <h1 class="i18n_updates_heading"></h1>

	55 <p class="i18n_updates_subheading"></p>

	56 </header>

	57

	58 <article class="feature-entry">

	59 <img class="feature-icon" src="/skin/updates/icon-rocket.svg" al t="rocket-icon"/>

	60 <p id="optionsPageNews" class="i18n_updates_optionsPageNews"></p >
	Thomas Greiner 2017/11/20 17:06:07 Coding style: "Separate words in ID and class name Coding style: "Separate words in ID and class names by a hyphen." See https://adblockplus.org/en/coding-style#html-css and https://google.github.io/styleguide/htmlcssguide.html#ID_and_Class_Name_Delim... martin 2017/11/26 17:08:40 Done. Show quoted text On 2017/11/20 17:06:07, Thomas Greiner wrote: > Coding style: "Separate words in ID and class names by a hyphen." > > See https://adblockplus.org/en/coding-style#html-css and > https://google.github.io/styleguide/htmlcssguide.html#ID_and_Class_Name_Delim... Done.
	61 </article>

	62

	63 <article class="feature-entry">

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

	65 <p id="block-ads-text" class="i18n_updates_blockAds"></p>

	66 </article>

	67

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

	69 <h2 class="i18n_updates_phone"></h2>

	70 <div class="feature-entry">

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

	72 <p class="i18n_updates_adblockBrowser"></p>

	73 </div>

	74 <div class="store-buttons">

	75 <a href="https://eyeo.to/adblockbrowser/ios/update-page" class ="store-button applestore-button">
	Thomas Greiner 2017/11/20 17:06:07 We use documentation links (see https://bitbucket. We use documentation links (see https://bitbucket.org/adblockplus/spec/src/3a5d8435c857976db8e4b58d18148504f4...) so we set any links using JavaScript. Looks like those links haven't been added to the spec yet though so we need to make sure to add them before we roll out the update. Example: https://hg.adblockplus.org/adblockplusui/file/tip/mobile-options.js#l435 martin 2017/11/26 17:08:39 I'm sort of confused as to how to use this. The `g Show quoted text On 2017/11/20 17:06:07, Thomas Greiner wrote: > We use documentation links (see > https://bitbucket.org/adblockplus/spec/src/3a5d8435c857976db8e4b58d18148504f4...) > so we set any links using JavaScript. > > Looks like those links haven't been added to the spec yet though so we need to > make sure to add them before we roll out the update. > > Example: > https://hg.adblockplus.org/adblockplusui/file/tip/mobile-options.js#l435 I'm sort of confused as to how to use this. The `getDocLink` basically selects the actual <a> DOM element. But then we set the `href` attribute to the `link` argument. Here's where I'm lost. Where is this "link" arg defined? Can't I just select the element and set its href to "http://something.something"? Out of curiosity - why are these utility functions necessary? Thomas Greiner 2017/11/27 16:15:47 No, the `getDocLink()` function gives you the URL Show quoted text On 2017/11/26 17:08:39, martin wrote: > I'm sort of confused as to how to use this. The `getDocLink` basically selects > the actual <a> DOM element. But then we set the `href` attribute to the `link` > argument. Here's where I'm lost. Where is this "link" arg defined? No, the `getDocLink()` function gives you the URL for a given ID, not the `<a>` element. Example: getDocLink("id", (url) => { element.href = url; }); So for the ID "releases", `"id"` would be `"releases"`, `url` would be `https://adblockplus.org/redirect?link=releases&lang=en-US` and `element` would be the `<a>` element. Show quoted text > Can't I just > select the element and set its href to "http://something.something"? Out of > curiosity - why are these utility functions necessary? It's possible to do it like this but there are two benefits to this approach: 1) By using adblockplus.org/redirect we can ensure that we don't link to web pages that no longer exist. Unlike with websites, links that we include in the extension are there forever so linking to our own website allows us to dynamically change the link destination without having to release an extension update. 2) Centralizing the creation of this link using `getDocLink()` allows us to avoid code duplication. martin 2017/12/05 14:37:29 Aaaaah! I get it now. Thanks for the explanation. Show quoted text On 2017/11/27 16:15:47, Thomas Greiner wrote: > On 2017/11/26 17:08:39, martin wrote: > > I'm sort of confused as to how to use this. The `getDocLink` basically selects > > the actual <a> DOM element. But then we set the `href` attribute to the `link` > > argument. Here's where I'm lost. Where is this "link" arg defined? > > No, the `getDocLink()` function gives you the URL for a given ID, not the `<a>` > element. > > Example: > getDocLink("id", (url) => > { > element.href = url; > }); > > So for the ID "releases", `"id"` would be `"releases"`, `url` would be > `https://adblockplus.org/redirect?link=releases&lang=en-US` and `element` would > be the `<a>` element. > > > Can't I just > > select the element and set its href to "http://something.something"? Out of > > curiosity - why are these utility functions necessary? > > It's possible to do it like this but there are two benefits to this approach: > > 1) By using adblockplus.org/redirect we can ensure that we don't link to web > pages that no longer exist. Unlike with websites, links that we include in the > extension are there forever so linking to our own website allows us to > dynamically change the link destination without having to release an extension > update. > > 2) Centralizing the creation of this link using `getDocLink()` allows us to > avoid code duplication. Aaaaah! I get it now. Thanks for the explanation. It makes sense now.
	76 <img src="skin/updates/appstore-bg.svg" alt="apple store but ton">

	77 </a>

	78 <a href="https://eyeo.to/adblockbrowser/android/update-page" c lass="store-button googleplay-button">

	79 <img src="skin/updates/googleplay-bg.svg" alt="google play s tore button">

	80 </a>

	81 </div>

	82 </article>

	83

	84 </div>

	85

	86 </section>

	87

	88 </div>

	89

	90 </main>

	91

	92 </body>

	93

	94 </html>

OLD	NEW

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