Issue 5540 - implement notification

skin/new-options.css

Index: skin/new-options.css
===================================================================
--- a/skin/new-options.css
+++ b/skin/new-options.css
@@ -1078,3 +1078,55 @@
 {
   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.

+  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
+{
+  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.

+  margin: 0rem 1rem;
+  background-color: transparent;
+}
+
+#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.

+}