| 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 |