Issue 5482 - Sidebar and about ABP dialog

skin/new-options.css

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
- * Copyright (C) 2006-2017 eyeo GmbH
+ * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
@@ skipping 17 matching lines @@
   src: url(fonts/SourceSansPro-Regular.woff);
   /* local("Ø") forces using no local font called Source Sans Pro */
   src: local("Ø"), url(fonts/SourceSansPro-Regular.woff) format("woff");
   font-weight: 400;
   font-style: normal;
 }
 
 @font-face
 {
   font-family: "Source Sans Pro";
-  src: url(fonts/SourceSansPro-Semibold.woff);
+  src: url(fonts/SourceSansPro-bold.woff);
   /* local("Ø") forces using no local font called Source Sans Pro */
-  src: local("Ø"), url(fonts/SourceSansPro-Semibold.woff) format("woff");
-  font-weight: 600;
+  src: local("Ø"), url(fonts/SourceSansPro-bold.woff) format("woff");
+  font-weight: 700;
   font-style: normal;
+}
+
+html
+{
+  font-size: 20px;
 }
 
 body
 {
   background-color: #F5F5F5;
   display: flex;
-  margin: 1.2em 0.3em;
+  margin: 1.2rem 0.3rem;
   font-family: "Source Sans Pro", sans-serif;
-  font-size: 20px;
+  font-weight: 300;
+  font-size: 1rem;
   color: #494949;
 }
 
 h1
 {
-  font-size: 48px;

[saroyanm, 2017/08/10 12:04:48]

I'll change font-sizes as soon they are updated in the style guide - as was
discussed in the meeting, the plan is to use EMs for the font-sizes as well.

https://bitbucket.org/adblockplus/spec/issues/55/use-ems-for-the-font-sizes-i...

[juliandoucette, 2017/08/16 23:42:31]

Acknowledged.

Thank you for pointing this out.

[saroyanm, 2017/08/18 12:44:28]

Done, but still I'm not really convinced with the specified sizes, this is
something we should run through and adjust with Jeen as soon we ship this.

[juliandoucette, 2017/08/21 13:06:39]

Acknowledged.

I agree. These sizes are huge. Perhaps we should ask the data team if we can
have something to inform Jeen about our average user's screen size.

+  font-size: 3rem;
   font-weight: 300;
 }
 
 h2
 {
-  font-size: 22px;
-  font-weight: 600;
+  font-size: 1.375rem;
+  font-weight: 700;
 }
 
 p
 {
   font-weight: 300;
 }
 
 [aria-hidden="true"]
 {
   display: none !important;
 }
 
 input[type="text"],
-textarea
+textarea,
+main
 {
   -webkit-box-sizing: border-box;
   -moz-box-sizing: border-box;
   box-sizing: border-box;
 }
 
 /*
-  Buttons
+  Buttons and links
  */
 
-button.primary,

[juliandoucette, 2017/08/16 23:42:32]

NIT: I prefer adding .button to <button>s and styling *just* .button here and
.button.primary below.

[saroyanm, 2017/08/18 10:23:13]

No, I don't think that we should have class button on the element <button> if
that what you mean.

I used .button to style non button elements as .button link in this case
"Contribute" which semantically is anchor, but looks like a "button".

[juliandoucette, 2017/08/18 11:17:04]

Acknowledged.

A <link> can look like a <button> just like a <button> can look like a <link>
(e.g. the button and link in the sidebar). And you can assign classes
accordingly or not {link: .button, button: .link}. I should point out that you
did not apply them accordingly in the sidebar because the <button> doesn't have
a .link class. But this is only a consistency NIT.

[saroyanm, 2017/08/18 12:44:28]

I agree, I thought you proposing to have button.button style, but now I see what
you mean, I think I made this more consitent now by introducing, button.link

[juliandoucette, 2017/08/21 13:06:39]

Acknowledged.

FWIW: I was originally proposing to have button.button. And I added another
different suggestion after you rejected my button.button NIT suggestion.

-button.secondary,
-.button.primary,
-.button.secondary
-{
-  padding: 0.8em 1.2em;
-  font-size: 18px;
-  font-weight: 600;
+button,
+.button
+{
+  padding: 0.8rem 1.2rem;
+  font-size: 1.125rem;
+  font-weight: 700;
   text-decoration: none;
   text-transform: uppercase;
+  cursor: pointer;
 }
 
 button.primary,
 .button.primary
 {
-  border: none;
+  border: 0px;
   color: #FFF;
   background-color: #0A9DD1;
-  cursor: pointer;
 }
 
 button.primary:hover,
 .button.primary:hover
 {
   box-shadow: inset 0 0 0 2px #005D80;
 }
 
 button.primary[disabled]
 {
   background-color: #5CBCE1;
 }
 
 button.secondary,
 .button.secondary
 {
   border: 1px solid #0A9DD1;
   color: #099CD0;
 }
 
 button.secondary:hover,
 .button.secondary:hover
 {
-  box-shadow: inset 0 0 0 1px #099CD0;
-}
-
-#about,
-a

[juliandoucette, 2017/08/16 23:42:32]

Did you mean to style all <a> here?

(It doesn't look like it)

[saroyanm, 2017/08/18 10:23:13]

Yes, I did, though I think some of the styles are misleading, don't remember why
I end up having them here as well.

I'll update it to look more generic.

[saroyanm, 2017/08/18 12:44:28]

Done.

-{
-  text-decoration: none;
-  color: #099CD0;
-  cursor: pointer;
-  font-size: 20px;
-  text-align: center;
+  box-shadow: inset 0 0 0 4px #099CD0;
 }
 
 button[role="checkbox"],
 #dialog-body .table button[role="checkbox"]
 {
   width: 18px;
   height: 18px;
   margin-top: 0px;
   -moz-margin-end: 20px;
   -webkit-margin-end: 20px;
   padding: 0px;
-  border: none;
+  border: 0px;
   background-color: transparent;
   background-position: -51px 0px;
 }
 
 button[role="checkbox"][aria-checked="true"],
 #dialog-body .table button[role="checkbox"][aria-checked="true"]
 {
   background-position: -68px 0px;
+}
+
+button.link
+{
+  border: 0px;
+  background-color: transparent;
+  font-weight: 300;
+  font-family: inherit;
+  color: #099CD0;
+  text-transform: none;
+  padding: 0.2rem;
 }
 
 /*
   Sidebar
  */
 
 #sidebar,
 #sidebar .fixed,
 [role="tablist"]
 {
-  width: 13.2em;
+  width: 13.2rem;
 }
 
 #sidebar
 {
   flex-shrink: 0; 
 }
 
 #sidebar .fixed
 {
-  top: 1.2em;
-  bottom: 0;
+  top: 1.2rem;
+  bottom: 0rem;
   height: auto;
   position: fixed;
 }
 
 #sidebar header
 {
   text-align: right;
-  margin-right: 2em;
+  margin-right: 2rem;
 }
 
 html[dir="rtl"] #sidebar header

[juliandoucette, 2017/08/16 23:42:32]

NIT: You could have just margin: 0 2em; above

[saroyanm, 2017/08/18 10:23:13]

I did it to save space, mostly for the translations. The fonts are to huge here
also considering that probably this is expected to reach the edge (0.3em from
edge, judging from the style-guide).

In general I think we should decrease the font-size, but I want to keep it for
later and don't stress style-guide updates, but rather revisit similar issues
later on with Jeen (including the active tab highlight question you have brought
up below).

[juliandoucette, 2017/08/18 11:17:04]

Acknowledged.

 {
-  margin-left: 2em;
+  margin-left: 2rem;
 }
 
 #sidebar header h1
 {
-  font-size: 24px;
-  margin: 0;
+  margin: 0rem;
+  font-size: 1.5rem;
+}
+
+#sidebar header h1 strong
+{
+  font-weight: 700;
 }
 
 #sidebar header p
 {
-  font-size: 44px;
-  margin: 0;

[juliandoucette, 2017/08/16 23:42:32]

NIT: I thought we'd agreed against unitless values?

[saroyanm, 2017/08/18 10:23:13]

Noted, will fix.

[saroyanm, 2017/08/18 12:44:28]

Done.

+  margin: 0rem;
+  font-size: 2.4rem;
 }
 
 html[dir="rtl"] #sidebar header
 {
   text-align: left;
 }
 
 #sidebar-logo

[juliandoucette, 2017/08/16 23:42:32]

NIT: It's kindof inconsistent that you are styling this image so specifically
and those nav and footer so generally.

[saroyanm, 2017/08/18 10:23:13]

I agree, I'll change.

[saroyanm, 2017/08/18 12:44:28]

On other hand, nav and footer are not unique elements, but sidebar-logo is, so
being specific in this case I think make sense. 
I keet the implementation if you agree.

[juliandoucette, 2017/08/21 13:06:39]

Acknowledged.

I agree. Good thinking.

 {
-  width: 3em;
-  margin-bottom: 0.9em;
+  width: 3rem;
+  margin-bottom: 0.7rem;
 }
 
 #sidebar nav,
 #sidebar footer
 {
-  margin: 1.4em 0;
+  margin: 1.4rem 0rem;
 }
 
 [role="tablist"]
 {
   list-style: none;
   margin: 0px;
   position: relative;
   padding: 0px;
-}
-
-[role="tablist"] li
+  font-size: 1.25rem;
+}
+
+[role="tablist"] li a
 {
   display: flex;
   margin-top: -1px;
-  padding: 1em 0px;
+  padding: 1rem 0.8rem;
   -moz-margin-end: -1px;
   -webkit-margin-end: -1px;
   -moz-margin-start: -1px;
   -webkit-margin-start: -1px;
   border-color: #CDCDCD transparent;
   border-style: solid;
   border-width: 1px;
-  font-weight: 300;
-  cursor: pointer;

[juliandoucette, 2017/08/16 23:42:31]

Is there a reason that you are using <spans> inside of <li> here instead of <a>?

[saroyanm, 2017/08/18 10:23:13]

No this is styles updated on top of old implementation of tabs, I agree that
having <a> instead of <span>s is correct here.

[saroyanm, 2017/08/18 12:44:28]

Done.

-}
-
-[role="tablist"] li a
-{
-  flex: 1;
+  text-decoration: none;
   color: inherit;
-  text-decoration: none;

[juliandoucette, 2017/08/16 23:42:32]

NIT/Note: You could default to none and add decoration to `#content a` or
otherwise.

[saroyanm, 2017/08/18 10:23:13]

I don't think we need text-decoration specified for any of a element according
to the styleguides/specs I'll update implementation defaulting that for <a> tag
in general

[saroyanm, 2017/08/18 12:44:29]

Done.

-}
-
-[role="tablist"] li a,
-[role="tablist"] li span,
-[role="tablist"] li::after
-{
-  margin: auto 0.8em;

[juliandoucette, 2017/08/16 23:42:32]

NIT: I don't think auto does anything here. If I am correct than it may be
misleading. 

[saroyanm, 2017/08/18 10:23:13]

It's not, well spotted.

[saroyanm, 2017/08/18 12:44:28]

Done.

-}
-
-[role="tablist"] li[role="tab"]:focus
+  cursor: pointer;
+}
+
+[role="tablist"] a[role="tab"]:focus
 {
   outline: none;
   text-shadow: 0px 0px 1px #888;

[juliandoucette, 2017/08/16 23:42:32]

NIT/Note: This doesn't look so hot on [lowdpi, chromium, debain]

[saroyanm, 2017/08/18 10:23:13]

I agree, I think we should enhance this with Jeen, update styleguide and the
implementation, as it's not specified in the style guide, anyway I think this
shouldn't be blocker for this review.
But thanks for heads-up.

[juliandoucette, 2017/08/18 11:17:04]

Acknowledged.

 }
 
-li[role="tab"][aria-selected]
+li a[role="tab"][aria-selected]
 {
   -moz-border-start-color: #CDCDCD;
   -webkit-border-start-color: #CDCDCD;
-  font-weight: 600;
+  font-weight: 700;
   background-color: #FFF;
 }
 
-footer

[juliandoucette, 2017/08/16 23:42:32]

Did you mean to style all <footer>s here?

(It doesn't look like it. The footer element is quite general and these styles
are quite specific.)

[saroyanm, 2017/08/18 10:23:13]

Currently we only have 1 footer, so I did it general. 
but I'll make this more specific.

[saroyanm, 2017/08/18 12:44:29]

Done.

+#sidebar footer
 {
   bottom: 0px;
   position: absolute;
   width: 100%;
 }
 
-footer p
+#sidebar footer p
 {
   display: flex;
   justify-content: center;
-  margin: 1.2em 0;
-}
-
-#about
-{
-  border:none;
-  background-color: transparent;
-  font-weight: 300;
-  font-family: inherit;
-}
-
-#content
+  margin: 1.2rem 0rem;
+}
+
+main
 {
   background-color: #FFFFFF;
   border: 1px solid #CDCDCD;
-  border-radius: 8px;
-  max-width: 46.3em;

[juliandoucette, 2017/08/16 23:42:32]

I think that you (or the specifier) may have made a miscalculation (e.g.
rounding and/or not counting borders). It looks like you (or the specifier) are
trying to reach a total (including the sidebar and spacing) max-width of 1280px;
and you are currently sitting at 1286px.

[saroyanm, 2017/08/18 10:23:12]

Yes, you are right this should be "border-box"

[saroyanm, 2017/08/18 12:44:28]

Done.

-  padding: 0px 2em 1.4em 2em;
-}
-
-#content h1
+  max-width: 46.3rem;
+  padding: 0px 2rem 1.4rem 2rem;
+}
+
+main > div
+{
+  display: none;
+}
+
+main h1
 {
   border-bottom: 1px solid #CDCDCD;
   margin: 0px 0px 24px 0px;
   padding: 40px 0px 16px 0px;
-}
-
-#content-wrapper
-{
-  position: relative;
-}
-
-#content-wrapper > div
-{
-  display: none;
 }
 
 body[data-tab|="general"] #content-general,
 body[data-tab|="advanced"] #content-advanced,
 body[data-tab|="whitelist"] #content-whitelist,
 body[data-tab|="help"] #content-help
 {
   display: block;
 }
 
@@ skipping 37 matching lines @@
 {
   padding-top: 0px;
   padding-bottom: 6px;
 }
 
   .table li.empty-placeholder
 {
   background-color: #F5F5F5;
 }
 
-.table li [data-single='visible'],
-.table li:first-child:nth-last-child(2) [data-single='hidden']
+.table li [data-single="visible"],
+.table li:first-of-type:last-of-type [data-single="hidden"]
 {
   display: none;
 }
 
-.table li:first-child:nth-last-child(2) [data-single='visible']
+.table li:first-of-type:last-of-type [data-single="visible"]
 {
   display: block;
 }
 
 .table label
 {
   vertical-align: top;
 }
 
 .table.cols
@@ skipping 21 matching lines @@
 
 .table button.delete:hover
 {
   background-position: -61px -51px;
 }
 
 .icon,
 button[role="checkbox"],
 #dialog-body .table button[role="checkbox"],
 .table button.delete,
-#content-help a::before,
 #dialog-close::before,
 #all-filter-lists-table .arrow,
 .context-menu .content a::before
 {
   background-image: url(options-sprite.png);
   display: inline-block;
 }
 
 .icon-add,
 .icon-update
@@ skipping 20 matching lines @@
   cursor: pointer;
 }
 
 #dialog-close::before
 {
   content: "";
   height: 12px;
   width: 12px;
 }
 
-#content-help a::before
+#dialog-body button::before
 {
   background-position: 0px -42px;
   content: "";
   cursor: pointer;
   height: 11px;
   vertical-align: middle;
   width: 7px;
   -moz-margin-end: 12px;
   -webkit-margin-end: 12px;
 }
@@ skipping 14 matching lines @@
 .controls > div
 {
   display: flex;
   position: relative;
 }
 
 .controls button,
 #dialog-close
 {
   display: flex;
-  border: none;
+  border: 0px;
   padding: 0px;
   align-items: center;
   background: none;
   cursor: pointer;
 }
 
 .controls button span:not(.icon)
 {
   -moz-margin-start: 16px;
   -webkit-margin-start: 16px;
   color: #3A7BA6;
   vertical-align: top;
+}
+
+/* 
+  Used for translatable screen reader only conten.
+  e.g.: Use instead of aria-label to avoid complex attribute value translation
+*/
+.sr-only
+{
+  position: absolute;
+  overflow: hidden;
+  clip: rect(0, 0, 0, 0);
+  width: 1px;
+  height: 1px;
+  margin: -1px;
+  padding: 0px;
+  border: 0px;
 }
 
 /*
   General tab content
 */
 
 #blocking-languages-dialog-table
 {
   border-bottom: none;
 }
@@ skipping 63 matching lines @@
   Advanced tab content
 */
 
 #all-filter-lists-table li.show-message .last-update,
 #all-filter-lists-table li:not(.show-message) .message,
 #custom-filters:not([data-mode="empty"]) #empty-custom-filters,
 #custom-filters[data-mode="empty"] #custom-filters-raw,
 #custom-filters:not([data-mode="write"]) #custom-filters-raw-controls,
 #custom-filters:not([data-mode="read"]) #custom-filters-edit,
 .state span,
-#acceptable-ads:not(.show-dnt-notification) #no-dnt
+#acceptable-ads:not(.show-dnt-notification) #dnt
 {
   display: none;
 }
 
 #all-filter-lists-table
 {
   display: inline-block;
 }
 
 #all-filter-lists-table
@@ skipping 213 matching lines @@
 
 div.context-menu > div a::before
 {
   vertical-align: middle;
   height: 16px;
   width: 16px;
 }
 
 div.context-menu > div a:last-child
 {
-  border: none;
+  border: 0px;
 }
 
 .context-menu .update-subscription::before
 {
   background-position: -38px -31px;
 }
 
 .context-menu .website::before
 {
   background-position: -33px -47px;
@@ skipping 46 matching lines @@
 
 div[role="tooltip"] .notes p
 {
   font-weight: 300;
 }
 
 /*
   Help tab content
 */
 
-#content-help
-{
-  counter-reset: section;
-}
-
-#content-help h1::before
-{
-  counter-increment: section;
-  content: counter(section) ". ";
-}
-
-#content-help a
-{
-  color: #3A7BA6;
+html:not([lang="zh"]) #social-chinese,
+html[lang="zh"] #social-general
+{
+  display: none;
+}
+
+#social ul
+{
+  list-style: none;
+}
+
+#social ul li
+{
   display: inline-block;
-  text-decoration: none;
-  -moz-margin-end: 16px;
-  -webkit-margin-end: 16px;
-  vertical-align: top;
-}
-
-#content-help a::before
-{
-  -moz-margin-end: 6px;
-  -webkit-margin-end: 6px;
-}
-
-#share-general:lang(zh),
-#share-chinese:not(zh)
-{
-  display: none;
+}
+
+#social ul li a
+{
+  display: block;
+  text-align: center;
+}
+
+#social ul li a::before
+{
+  display: block;
+  margin: 0em auto;
+  width: 2.5rem;
+  height: 2.5rem;
+  content: "";
+  background-color: #099DD1;
+}
+
+#twitter::before
+{
+  -webkit-mask: url("social/twitter.svg");
+  mask: url("social/twitter.svg");
+}
+
+#facebook::before
+{
+  -webkit-mask: url("social/facebook.svg");
+  mask: url("social/facebook.svg");
+}
+
+#google-plus::before
+{
+  -webkit-mask: url("social/googleplus.svg");
+  mask: url("social/googleplus.svg");
 }
 
 /*
   Dialog
 */
 
 #dialog-background
 {
   display: none;
   position: fixed;
@@ skipping 106 matching lines @@
   margin-top: 10px;
   margin-bottom: 20px;
   word-wrap: break-word;
 }
 
 body:not([data-dialog="about"]) #dialog-title-about,
 body:not([data-dialog="about"]) #dialog-content-about,
 body:not([data-dialog="custom"]) #dialog-title-custom,
 body:not([data-dialog="custom"]) #dialog-content-custom,
 body:not([data-dialog="language-add"]) #dialog-title-language-add,
-body:not([data-dialog="language-add"]) #dialog-content-language-add,
 body:not([data-dialog="language-change"]) #dialog-title-language-change,
-body:not([data-dialog="language-change"]) #dialog-content-language-change,
+body:not([data-dialog="language-add"]):not([data-dialog="language-change"]) #dialog-content-language-add,
+body:not([data-dialog="language-add"]) #dialog-body button.add,
+body:not([data-dialog="language-change"]) #dialog-body button.change,
 body:not([data-dialog="predefined"]) #dialog-title-predefined,
 body:not([data-dialog="predefined"]) #dialog-content-predefined,
 body:not([data-dialog]) #dialog
 {
   display: none;
 }
+
+/*
+  Notification
+*/
+
+#notification
+{
+  position: fixed;
+  top: 0rem;
+  left: 0rem;
+  display: flex;
+  padding: 1rem 1.9rem;
+  width: 100%;
+  box-sizing: border-box;
+  opacity: 0.8;
+  font-size: 1rem;
+  color: #099CD0;
+  background-color: #E1F2FA;
+}
+
+#notification strong
+{
+  flex: 1;
+  text-align: center;
+}
+
+#hide-notification
+{
+  border: 0rem;
+  padding: 0rem;
+  margin: 0rem 1rem;
+  background-color: transparent;
+  cursor: pointer;
+}
+
+#hide-notification::after
+{
+  content: "";
+  display: block;
+  height: 1rem;
+  width: 1rem;
+  border: 0rem;
+  background-color: #099DD1;
+  -webkit-mask: url(delete.svg);
+  mask: url(delete.svg);
+}
+
+#hide-notification:hover::after
+{
+  background-color: #5CBCE1;
+}