Left: | ||
Right: |
OLD | NEW |
---|---|
(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> | |
OLD | NEW |