Issue 5943 - Implement Updates Page for Adblock Plus extension

skin/updates.css

-@font-face 
-{
-    font-family: "Source Sans Pro";

[Thomas Greiner, 2017/11/03 17:40:01]

Coding style: "Indentation: Two spaces per logic level"

See
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style...

[martin, 2017/11/06 16:05:45]

Done.

-    font-style: normal;
-    font-weight: 400;
-    font-stretch: normal;
-    src: local ("Ø"),
-    url(fonts/SourceSansPro-Regular.woff) format("woff");
-}
-
 @font-face
 {
-    font-family: "Source Sans Pro";
-    font-style: bold;
-    font-weight: 600;
-    font-stretch: normal;
-    src: local ("Ø"),
-    url(fonts/SourceSansPro-bold.woff) format("woff");
+  font-family: "Source Sans Pro";
+  font-style: normal;
+  font-weight: 400;
+  font-stretch: normal;
+  src: local ("Ø"),
+  url(fonts/SourceSansPro-Regular.woff) format("woff");
+}
+
+@font-face
+{
+  font-family: "Source Sans Pro";
+  font-style: bold;
+  font-weight: 600;
+  font-stretch: normal;
+  src: local ("Ø"),
+  url(fonts/SourceSansPro-bold.woff) format("woff");
 }
 
 html
 {
-    font-family: "Source Sans Pro", Helvetica, Arial, sans-serif;
+  font-family: "Source Sans Pro", Helvetica, Arial, sans-serif;
+  -webkit-font-smoothing: antialiased;
 }
 
 body
 {
-    margin: 0;
-    padding: 0;
+  margin: 0;
+  padding: 0;
 }
 
 #container
 {
-    display: flex;
-    flex-direction: row;
-    align-items: stretch;
-}
-
-@media(max-width: 960px)

[Thomas Greiner, 2017/11/03 17:40:00]

I'd be curious to hear what Julian's opinion is on this because usually we place
all conditional styles at the bottom to have a better overview of the
differences for certain screen sizes.

[martin, 2017/11/06 16:05:43]

Moved the query to the bottom of the file.

-{
-    #container
-    {
-        flex-direction: column;
-    }
+  display: flex;
+  flex-direction: row;
+  align-items: stretch;
 }
 
 .column
 {
   height: 100vh;
-  display: flex; 
+  display: flex;
   align-items: center;
   justify-content: center;
   flex: 1;
 }
 
-#graphic-column
-{
-  background: #8DC446;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: I'd recommend avoiding shorthand properties if they don't add any value
because it not only overrides "background-color" but also all other
"background-*" properties that have previously been defined.

Also applies to other occurrences in this file.

[martin, 2017/11/06 16:05:45]

Done.

-
-}
-
-.graphic-column
-{
-    background: #8DC446;
+ #graphic-column,
+ .graphic-column
+{
+  background-color: #8DC446;
 }
 
 #content
 {
-    padding: 0 3em;
-    max-width: 760px;
-}
-
-#content a {
-    color: #C70D2C;
-    text-decoration: none;
-}
-
-#content a:hover {
-    text-decoration: underline;
+  padding: 0 3em;
+  max-width: 760px;
+}
+
+#content a
+{
+  color: #C70D2C;
+  text-decoration: none;
+}
+
+#optionsPageLink
+{
+  color: red;
+}
+
+/* Needed in order to achieve standard behavior 
+for links, due to lacking href attribute */
+#content a:hover
+{
+  text-decoration: underline;
+  cursor: pointer;
 }
 
 #content header
 {
-    margin: 0 0 2em 5em;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: This may produce unexpected results for right-to-left languages.

[martin, 2017/11/06 16:05:45]

Done.

+  margin: 0 0 2em 5em;
+}
+
+html[dir="rtl"] #content header
+{
+  margin: 0 5em 2em 0;
 }
 
 #content header p {
-    margin: 0;
-    opacity: .5;

[Thomas Greiner, 2017/11/03 17:40:01]

Coding style: "Don't omit the optional leading 0 for decimal numbers."

See https://adblockplus.org/en/coding-style#html-css

[martin, 2017/11/06 16:05:44]

Done.

[martin, 2017/11/06 16:05:45]

Done.

-    font-weight: normal;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: For consistency, we use numeric values for the font weight.

[martin, 2017/11/06 16:05:44]

Done.

-    font-size: 1.2em;
+  margin: 0;
+  opacity: 0.5;
+  font-weight: 400;
+  font-size: 1.2em;
 }
 
 #content header h1
 {
-    margin: 0;
+  margin: 0;
 }
 
 .custom-feature-entry
 {
-    margin-top: 2em;
-}
-
-.custom-feature-entry h2 {
-    margin: 0 0 0 3.4em;
-}
-
-.update-graphic-container 
-{
-    width: 560px;
-    height: 460px;
-    background: url('/skin/updates-page/base-graphic.svg');

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: Note that we've been using relative paths on other pages so I'd
recommend changing the paths throughout this file for consistency in case the
absolute path changes in the future.

[Thomas Greiner, 2017/11/03 17:40:00]

Suggestion: Mind using the same name for the directory as for the update page
itself (i.e. "updates" instead of "updates-page") for consistency? Otherwise
this may bite us later on.

[Thomas Greiner, 2017/11/03 17:40:01]

Coding style: "Do not use quotation marks in URI values (url())."

See https://google.github.io/styleguide/htmlcssguide.html#CSS_Quotation_Marks

[martin, 2017/11/06 16:05:43]

Done.

[martin, 2017/11/06 16:05:44]

Done.

[martin, 2017/11/06 16:05:44]

Done.

-    position: relative;
-    display: flex;
-    align-items: center;
-    background-repeat: no-repeat;
-    background-size: cover;
+  margin-top: 2em;
+}
+
+.custom-feature-entry h2
+{
+  margin: 0 0 0 3.4em;
+}
+
+html[dir="rtl"] .custom-feature-entry h2
+{
+  margin: 0 3.4em 0 0;
+}
+
+.update-graphic-container
+{
+  display: flex;
+  align-items: center;
+  position: relative;
+  width: 560px;
+  height: 460px;
+  background-image: url(./updates/base-graphic.svg);
+  background-repeat: no-repeat;
+  background-size: cover;
 }
 
 .update-graphic-container img
 {
-    display: block;
-    width: 162px;
-    height: 162px;
+  display: block;
+  width: 162px;
+  height: 162px;
 }
 
 .update-graphic-content
 {
-    position: absolute;
-    top: 65px;
-    left: 100px;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: This may produce unexpected results for right-to-left languages.

[martin, 2017/11/06 16:05:45]

Done.

[Thomas Greiner, 2017/11/14 12:25:38]

How did you address this?

[martin, 2017/11/20 11:47:28]

I checked it out and I guess it looks all right in "rtl". Let me know if you had
something else in mind.

[Thomas Greiner, 2017/11/20 17:06:03]

I looked at the page now and see that this is not an issue simply because the
background image isn't mirrored. So you're right, it should be fine.

-    display: flex;
-    align-items: center;
+  display: flex;
+  align-items: center;
+  position: absolute;
+  top: 65px;
+  left: 100px;
 }
 
 .version-details
 {
-    margin: 0 0 0 1em;
+  margin: 0 1em 0 1em;
 }
 
 .version-details h2
 {
-    margin: 0;
-}
-
-.version-details span
-{
-    color: #bebebe;
+  margin: 0;
 }
 
 .feature-entry
 {
-    display: flex;
-    align-items: center;
-    padding: 0 1em;
+  display: flex;
+  align-items: center;
+  padding: 0 1em;
 }
 
 .feature-entry img
 {
-    width: 50px;
-    height: 50px;
-    display: block;
-    margin: 0 1em 0 0;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: This may produce unexpected results for right-to-left languages.

[martin, 2017/11/20 11:47:28]

Done.

+  width: 50px;
+  height: 50px;
+  display: block;
+  margin: 0 1em 0 0;
+}
+
+html[dir="rtl"] .feature-entry img
+{
+  margin: 0 0 0 1em;
+}
+
+#block-ads-text > strong
+{
+  text-decoration: line-through;
+  font-weight: normal;
 }
 
 .store-buttons
 {
-    width: 100%;
-    hegiht: 58px;

[Thomas Greiner, 2017/11/03 17:40:01]

Typo: Replace "hegiht" with "height"

[martin, 2017/11/06 16:05:43]

Done.

-    margin-left: 5em;

[Thomas Greiner, 2017/11/03 17:40:00]

Detail: This may produce unexpected results for right-to-left languages.

[martin, 2017/11/06 16:05:44]

Done.

+  height: 58px;
+  margin: 0 0 0 5em;
+}
+
+html[dir="rtl"] .store-buttons
+{
+  margin: 0 5em 0 0;
 }
 
 .store-button
 {
-    display: block;
-    width: 180px;
-    height: 58px;
-    border-radius: 6px;
-    background: #000;
-    float: left;

[Thomas Greiner, 2017/11/03 17:40:01]

Detail: This may produce unexpected results for right-to-left languages.

In general, I'd recommend avoiding floats as much as possible since they can
lead to all sorts of trouble.

[martin, 2017/11/06 16:05:45]

Done.

-    margin-bottom: 1em;
-}
-
-.store-button > img 
-{
-    height: 54px;
+  display: block;
+  height: 58px;
+  border-radius: 6px;
+  background-color: #000;
+  float: left;
+  margin-bottom: 1em;
+}
+
+html[dir="rtl"] .store-button
+{
+  float: right;
+}
+
+.store-button > img
+{
+  height: 100%;
 }
 
 .applestore-button
 {
-    margin-right: 1em;
-}
-
-[dir="rtl"] .appstore-button
-{
-    margin-right: 0;
-    margin-left: 1em;
-}
+  margin-right: 1em;
+}
+
+html[dir="rtl"] .applestore-button
+{
+  margin-right: 0;
+  margin-left: 1em;
+}
+
+@media(max-width: 960px)
+{
+  #container
+  {
+    flex-direction: column;
+  }
+}
+
+@media(max-width: 480px)
+{
+  #content header
+  {
+    margin: 2em 0 2em 5em;
+  }
+
+  .column
+  {
+    align-items: flex-start;
+  }
+
+  html[dir="rtl"] .applestore-button
+  {
+    margin: 0 0 1em 0;
+  }
+}