Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: index.html

Issue 29646555: Issue 6210 - Implement Subscription (Double-opt-in) template for newsletter (Closed)
Patch Set: Update privacy policy link Created Dec. 22, 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
OLDNEW
(Empty)
1 <!doctype html>
2 <html lang="en" dir="ltr">
3 <head>
4 <!-- Website Defaults Standard Metadata -->
juliandoucette 2017/12/22 14:34:30 TOL: Is <!-- website-defaults/includes/meta/standa
ire 2017/12/22 16:29:37 Done.
5 <meta charset="utf-8">
6 <meta http-equiv="x-ua-compatible" content="ie=edge">
7 <meta name="viewport" content="width=device-width, initial-scale=1">
8 <meta charset="utf-8">
juliandoucette 2017/12/22 14:34:30 Duplicate.
ire 2017/12/22 16:29:37 Done.
9 <title>Adblock Plus newsletter signup form</title>
10 <meta name="description" content="A newsletter sign up form, so users can stay up to date with all things related to Adblock Plus.">
11
12 <!-- Resources -->
13 <link rel="stylesheet" type="text/css" href="css/main.min.css">
14 </head>
15 <body>
16 <div class="outer-container">
juliandoucette 2017/12/22 14:34:28 NIT: This has more to do with vertical alignment t
ire 2017/12/22 16:29:36 I don't think the word ".container" refers horizon
17 <div class="container phablet-width">
18
19 <header id="page-header">
20 <figure>
juliandoucette 2017/12/22 14:34:30 TOL: I think this markup makes sense. But I'm not
ire 2017/12/22 16:29:36 It makes sense to me. I'll look into the figcaptio
21 <img src="img/abp-logo.png" srcset="img/abp-logo.svg 2x" alt="ABP insi de red octagon">
juliandoucette 2017/12/22 14:34:29 NIT: The one that you are using looks better than
ire 2017/12/22 16:29:37 I'm actually using the logo from the adblockplusui
22 <figcaption>Adblock <strong>Plus</strong></figcaption>
23 </figure>
24 <h1 class="lead"><strong>Sign up for the Adblock Plus newsletter</strong ></h1>
juliandoucette 2017/12/22 14:34:28 NIT: I think heading emphasis is enough. We can ma
juliandoucette 2017/12/22 14:34:30 NIT: This heading looks larger than that seen in t
ire 2017/12/22 16:29:37 I thought it looked better this way but I'll stick
ire 2017/12/22 16:29:38 Done.
25 <p>Stay up to date with all things Adblock Plus</p>
26 </header>
27
28 <form id="sign-up">
juliandoucette 2017/12/22 14:34:28 This form doesn't actually work. I can't approve u
ire 2017/12/22 16:29:36 As I mentioned in the review description, this was
29 <label for="email">Email Address *</label>
juliandoucette 2017/12/22 14:34:28 NIT: There seems to be more space below these in t
ire 2017/12/22 16:29:37 Done.
30 <input type="email" id="email" name="email" required>
31 <span class="error-message">Please enter a valid email address</span>
juliandoucette 2017/12/22 14:34:29 NIT/Suggest: <div> instead of <span> to make it cl
ire 2017/12/22 16:29:36 Done.
32
33 <label for="fullname">Full name</label>
34 <input type="text" id="fullname" name="fullname">
35
36 <button type="submit" class="secondary">Sign Up</button>
37 </form>
38 <footer id="page-footer">
39 <a href="https://eyeo.com/en/privacy" target="_blank">Privacy Policy</a>
40 </footer>
41 </div>
42 </div>
43 </body>
44 </html>
OLDNEW
« no previous file with comments | « img/abp-logo.svg ('k') | package.json » ('j') | scss/_content.scss » ('J')

Powered by Google App Engine
This is Rietveld