firstRun.html - Issue 29565721: No Issue - Implemented first run page

Side by Side Diff: firstRun.html

Issue 29565721: No Issue - Implemented first run page Base URL: https://hg.adblockplus.org/adblockplusui/

Patch Set: Created Oct. 5, 2017, 1:47 p.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
1 <!DOCTYPE html>	1 <!DOCTYPE html>

2 <!--	2 <!--

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

4 - Copyright (C) 2006-present eyeo GmbH	4 - Copyright (C) 2006-present eyeo GmbH

5 -	5 -

6 - Adblock Plus is free software: you can redistribute it and/or modify	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	7 - it under the terms of the GNU General Public License version 3 as

8 - published by the Free Software Foundation.	8 - published by the Free Software Foundation.

9 -	9 -

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

(...skipping 11 matching lines...) Expand all Loading...
22 <meta charset="utf-8" />	22 <meta charset="utf-8" />

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

24 <link type="text/css" href="skin/common.css" rel="stylesheet" />	24 <link type="text/css" href="skin/common.css" rel="stylesheet" />

25 <link type="text/css" href="skin/firstRun.css" rel="stylesheet"/>	25 <link type="text/css" href="skin/firstRun.css" rel="stylesheet"/>

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

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

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

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

30 <script type="text/javascript" src="firstRun.js"></script>	30 <script type="text/javascript" src="firstRun.js"></script>

31 </head>	31 </head>

	32

32 <body>	33 <body>
	juliandoucette 2017/10/08 13:18:10 NIT: You should wrap the content currently on this NIT: You should wrap the content currently on this page with <main> (See https://www.w3.org/TR/html5/grouping-content.html#the-main-element for explanation) martin 2017/10/16 09:09:08 Done. Show quoted text On 2017/10/08 13:18:10, juliandoucette wrote: > NIT: You should wrap the content currently on this page with <main> > > (See https://www.w3.org/TR/html5/grouping-content.html#the-main-element for > explanation) Done.
33	34

34

35 <header>	35 <header>

36 <img id="logo">	36 <img src="./skin/icons/first-run/icon-checkmark-header.svg" alt="checkma rk icon">
	juliandoucette 2017/10/08 13:18:10 NIT: Alt text should be translated (The same appl NIT: Alt text should be translated (The same applies elsewhere) juliandoucette 2017/10/08 13:18:10 NIT: "./" is unnecessary here (The same applies e NIT: "./" is unnecessary here (The same applies elsewhere) martin 2017/10/16 09:09:08 I honestly don't know how to do that with the curr Show quoted text On 2017/10/08 13:18:10, juliandoucette wrote: > NIT: Alt text should be translated > > (The same applies elsewhere) I honestly don't know how to do that with the current translation setup. If we were using some sort of templating I could have gone for something like alt={{ %translation-string }}, however I'm not sure how to translate alt tags currently :/ saroyanm 2017/10/16 10:22:59 text inside of attributes can't be easily translat Show quoted text On 2017/10/16 09:09:08, martin wrote: > On 2017/10/08 13:18:10, juliandoucette wrote: > > NIT: Alt text should be translated > > > > (The same applies elsewhere) > > I honestly don't know how to do that with the current translation setup. If we > were using some sort of templating I could have gone for something like alt={{ > %translation-string }}, however I'm not sure how to translate alt tags currently > :/ text inside of attributes can't be easily translated, it fact it's not even translated for new options page. I'll suggest to keep the way it is right now and handle that in separate issue, not to over-complicate this review. martin 2017/10/16 14:36:00 Done. Show quoted text On 2017/10/16 09:09:08, martin wrote: > On 2017/10/08 13:18:10, juliandoucette wrote: > > NIT: Alt text should be translated > > > > (The same applies elsewhere) > > I honestly don't know how to do that with the current translation setup. If we > were using some sort of templating I could have gone for something like alt={{ > %translation-string }}, however I'm not sure how to translate alt tags currently > :/ Done.
37 <h1 id="title-main" class="i18n_firstRun_title"></h1>	37 <h1 class="i18n_firstRun_title"></h1>

	38 <p class="i18n_firstRun_subtitle"></p>

38 </header>	39 </header>

39	40

40 <section id="filterlistsReinitializedWarning" class="i18n_firstRun_filterlis tsReinitializedWarning warning" hidden="true"></section>	41 <div class="container content">

41	42

42 <div id="content">	43 <div class="row">

43 <section id="general">

44 <div id="acceptable-ads-block" class="block">

45 <h2 class="i18n_firstRun_acceptableAdsHeadline"></h2>

46 <p id="acceptable-ads-explanation" class="i18n_firstRun_acceptableAdsE xplanation"></p>

47 </div>

48 <div id="abb-promotion-block" class="block">

49 <h2 class="i18n_firstRun_abbPromotionHeadline"></h2>

50 <a href="https://adblockbrowser.org/?ref=frp">

51 <div>

52 <img src="skin/abb-logo.png" alt="Adblock Browser logo" />

53 </div>

54 <div>

55 <div class="i18n_firstRun_abbButtonTitle title"></div>

56 <div class="i18n_firstRun_abbButtonSubtitle subtitle"></div>

57 </div>

58 </a>

59 </div>

60 </section>

61	44

62	45 <div class="column one-third">
	juliandoucette 2017/10/08 13:18:10 NIT: These (columns) should be <article>s (See ht NIT: These (columns) should be <article>s (See https://www.w3.org/TR/html5/sections.html#the-article-element for explanation) (<section> vs <article> is debatable here. I'm qualifying this as "an interactive widget or gadget, or any other independent item of content." because their contents are reusable and ~different-enough (to be "independent") from each other (and the outer section).) martin 2017/10/16 09:09:08 Done. Show quoted text On 2017/10/08 13:18:10, juliandoucette wrote: > NIT: These (columns) should be <article>s > > (See https://www.w3.org/TR/html5/sections.html#the-article-element for > explanation) > > (<section> vs <article> is debatable here. I'm qualifying this as "an > interactive widget or gadget, or any other independent item of content." because > their contents are reusable and ~different-enough (to be "independent") from > each other (and the outer section).) Done.
63 <section id="share">	46 <img src="./skin/icons/first-run/icon-checkmark.svg" alt="checkmark ic on">

64 <h2 id="share-headline" class="i18n_firstRun_share_headline"></h2>	47 <h2 class="i18n_firstRun_columnOneTitle"></h2>
	juliandoucette 2017/10/08 13:18:10 NIT: I think that you should wrap the <img> and th NIT: I think that you should wrap the <img> and the <h2> in a <header> to communicate that the image is part of the heading. martin 2017/10/16 09:09:09 Done. Show quoted text On 2017/10/08 13:18:10, juliandoucette wrote: > NIT: I think that you should wrap the <img> and the <h2> in a <header> to > communicate that the image is part of the heading. Done.
65	48 <p id="first-column-description" class="i18n_firstRun_columnOneDescrip tion"></p>

66 <div class="block">	49 <a id="options-trigger" class="i18n_firstRun_configure-button button-p rimary" href="#"></a>

67 <a id="donate" class="i18n_firstRun_donate" target="_blank"></a>

68 <span id="donate-label" class="i18n_firstRun_donate_label"></span>

69 </div>	50 </div>

70	51

71 <div class="block">	52 <div class="column one-third">

72 <div id="share-general" class="share-buttons">	53 <img src="./skin/icons/first-run/icon-rocket.svg" alt="rocket icon">

73 <a id="share-facebook" href="https://www.facebook.com/adblockplus" t arget="_blank" data-script="https://facebook.com/plugins/like.php?">	54 <h2 class="i18n_firstRun_columnTwoTitle"></h2>

74 </a>	55 <p class="i18n_firstRun_columnTwoDescription"></p>

75 <a id="share-twitter" href="https://twitter.com/adblockplus" target= "_blank" data-script="https://platform.twitter.com/widgets/">	56 <div class="store-buttons">

76 </a>	57 <a href="" class="store-button applestore-button"></a>
	juliandoucette 2017/10/08 13:18:10 NIT: There is no text in this button You could us NIT: There is no text in this button You could use [a.title, a > img.alt, a > span.sr-only] (if you know what I mean?) (The same applies elsewhere.) martin 2017/10/16 09:09:09 Soo... I think what you mean is that instead of Show quoted text On 2017/10/08 13:18:10, juliandoucette wrote: > NIT: There is no text in this button > > You could use [a.title, a > img.alt, a > span.sr-only] (if you know what I > mean?) > > (The same applies elsewhere.) Soo... I think what you mean is that instead of setting a "background-image" to an empty <a>, I should nest an <img> inside and set the src of that? Or? juliandoucette 2017/10/16 10:00:38 ~Yes. I suggested the following solutions: 1. Pl Show quoted text On 2017/10/16 09:09:09, martin wrote: > On 2017/10/08 13:18:10, juliandoucette wrote: > > NIT: There is no text in this button > > > > You could use [a.title, a > img.alt, a > span.sr-only] (if you know what I > > mean?) > > > > (The same applies elsewhere.) > > Soo... I think what you mean is that instead of setting a "background-image" > to an empty <a>, I should nest an <img> inside and set the src of that? Or? ~Yes. I suggested the following solutions: 1. Place a title attribute on the <a> 2. Place an <img> with an alt attribute in the <a> 3. Place a <span> that is screenreader-only in the <a> Of which, solution #2 is the most correct. (An even better solution would be to create the entire button using HTML/CSS and an image icon with alt text.) martin 2017/10/16 14:36:00 Done. I went for option 2. Show quoted text On 2017/10/16 10:00:38, juliandoucette wrote: > On 2017/10/16 09:09:09, martin wrote: > > On 2017/10/08 13:18:10, juliandoucette wrote: > > > NIT: There is no text in this button > > > > > > You could use [a.title, a > img.alt, a > span.sr-only] (if you know what I > > > mean?) > > > > > > (The same applies elsewhere.) > > > > Soo... I think what you mean is that instead of setting a "background-image" > > to an empty <a>, I should nest an <img> inside and set the src of that? Or? > > ~Yes. > > I suggested the following solutions: > > 1. Place a title attribute on the <a> > 2. Place an <img> with an alt attribute in the <a> > 3. Place a <span> that is screenreader-only in the <a> > > Of which, solution #2 is the most correct. > > (An even better solution would be to create the entire button using HTML/CSS and > an image icon with alt text.) Done. I went for option 2.
77 <a id="share-gplus" href="https://www.google.com/+AdblockPlus" targe t="_blank" data-script="https://apis.google.com/se/0/_/+1/fastbutton?">	58 <a href="" class="store-button googleplay-button"></a>

78 </a>	59 </div>

79 </div>	60 </div>

80	61

81 <!-- Chinese social networks -->	62 <div class="column one-third">

82 <div id="share-chinese" class="share-buttons">	63 <img src="./skin/icons/first-run/icon-lock.svg" alt="padlock icon">

83 <a id="share-renren" href="http://www.renren.com/601651969" target=" _blank">	64 <h2 class="i18n_firstRun_columnThreeTitle"></h2>

84 </a>	65 <p id="third-column-description" class="i18n_firstRun_columnThreeDes cription"></p>

85 <a id="share-weibo" href="http://e.weibo.com/adblockplus/" target="_ blank">

86 </a>

87 </div>

88 <span class="i18n_firstRun_share"></span>

89 </div>	66 </div>

90 </section>	67

	68 </div>

91	69

92 </div>	70 </div>

93	71

94 <footer>

95 <a id="contributors" class="i18n_firstRun_contributor_credits"></a>

96 </footer>

97 </body>	72 </body>

	73

98 </html>	74 </html>

OLD	NEW

« no previous file with comments | « no previous file | firstRun.js » ('j') | skin/firstRun.css » ('J')