Issue 5540 - implement notification

skin/new-options.css

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * 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
@@ skipping 407 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 27 matching lines @@
   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 364 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 136 matching lines @@
 {
   display: none;
 }
 
 /*
   Notification
 */
 
 #notification
 {
-  position: absolute;

[ire, 2017/08/24 10:08:44]

I think this should be `position: fixed`. 

Although unlikely, the user might be slightly scrolled down on the page (e.g. if
they have a long list of whitelisted websites) and will not be able to see the
notification.

[saroyanm, 2017/08/24 14:18:26]

Good point!

[saroyanm, 2017/08/24 18:40:54]

Done.

+  position: fixed;
   top: 0rem;
   left: 0rem;
   display: flex;
   padding: 1rem 1.9rem;
   width: 100%;
   box-sizing: border-box;

[ire, 2017/08/24 10:08:44]

Not sure if this will be addressed by implementing issue 5543, but this
box-sizing style seems like it should be applied to all elements

[saroyanm, 2017/08/24 14:18:26]

Not sure if we should apply this to all elements, I know that the practice
exists.
If it appears that we need this on all elements will apply globally in separate
review.
But the reason why I did it so is because the scrollbar that was appearing when
I was using padding as this is 100% width absolute item.

[ire, 2017/08/25 09:59:45]

Acknowledged.

   opacity: 0.8;
   font-size: 1rem;
-  font-weight: 600;

[ire, 2017/08/24 10:08:44]

Can this bold style be applied by using a <strong> element instead?

[saroyanm, 2017/08/24 14:18:26]

Yes, I'll use <strong> instead

[saroyanm, 2017/08/24 18:40:54]

Done.

   color: #099CD0;
   background-color: #E1F2FA;

[ire, 2017/08/24 10:08:44]

(I think this may have been discussed already but) this color to
background-color contrast ratio is just 2.7

[saroyanm, 2017/08/24 14:18:26]

I agree, that the contrast needs to be adjusted.

but that shouldn't be blocker for the review.

So either I can bring that up during the next meeting, as I collecting similar
questions. So we will have something ready to show as well. Or maybe you can
create a bitbucket issue explaining the contrast.

[ire, 2017/08/25 09:59:45]

It's fine to bring up during the next meeting since it'll likely affect other
parts of the site as well

 }
 
-#notification span
+#notification strong
 {
   flex: 1;
   text-align: center;
 }
 
 #hide-notification
 {
   border: 0rem;
-  padding: 0rem;e

[ire, 2017/08/24 10:08:45]

There's a stray "e" here :p

[saroyanm, 2017/08/24 14:18:26]

:D 

[saroyanm, 2017/08/24 18:40:54]

Done.

+  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;
-  cursor: pointer;

[ire, 2017/08/24 10:08:44]

I think the `cursor: pointer` should be on the button itself, not the image
only. If you hover the mouse around the area of the image, which is still the
button, the cursor changes even though it's still a clickable area.

[saroyanm, 2017/08/24 14:18:26]

Right. I'll change that.

[saroyanm, 2017/08/24 18:40:54]

Done.

   -webkit-mask: url(delete.svg);
   mask: url(delete.svg);
 }
 
 #hide-notification:hover::after
 {
   background-color: #5CBCE1;

[ire, 2017/08/24 10:08:45]

(I think this was also discussed before but) the change in colour here is too
insignificant, it's unnoticeable. Plus this colour's contrast ratio is too low. 

[saroyanm, 2017/08/24 14:18:26]

Yes, I agree, same as above question. I'll collect all similar cases. Adjusting
colors is easy and probably question of minutes, but discussions and agreements
usually takes time.

So we probably will have separate issue addressing all contrast issues, but
currently let's be consistent with style guide.

 }