Issue 5628 - Implement main styles

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 13 matching lines @@
   font-weight: 300;
   font-style: normal;
 }
 
 @font-face
 {
   font-family: "Source Sans Pro";
   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-bold.woff);
   /* local("Ø") forces using no local font called Source Sans Pro */
   src: local("Ø"), url(fonts/SourceSansPro-bold.woff) format("woff");
   font-weight: 700;
   font-style: normal;
 }
 
 html
 {
   font-size: 16px;
 }
 
 body
 {
   background-color: #F3F3F3;
   display: flex;
   margin: 1.2rem 0.3rem;
   font-family: "Source Sans Pro", sans-serif;
-  font-weight: 400;

[juliandoucette, 2017/09/12 22:45:36]

NIT: I think 400 is the default across browsers.

[saroyanm, 2017/09/13 15:26:37]

Acknowledged.

[saroyanm, 2017/09/13 16:47:58]

Done.

   font-size: 1.25rem;
   color: #494949;
 }
 
 h1
 {
   font-size: 3rem;
   font-weight: 300;
 }
 
@@ skipping 114 matching lines @@
 }
 
 button[role="checkbox"][aria-checked="true"].toggle
 {
   background: url(icons/toggle.svg#on);
 }
 
 button[role="checkbox"].toggle
 {
   background-position: initial;
-  width: 2.3rem;
+  width: 2.25rem;
   height: 1.3rem;
 }
 
 button[role="checkbox"][disabled].toggle
 {
   background-image: none;
 }
 
 button.delete::before
 {
   -webkit-mask: url(icons/trash.svg);

[saroyanm, 2017/09/06 19:02:56]

Note: I'm using masks currently as it's the most quick and easy way to change
the color on different states. In general this will be changed and be consistent
with the implementation of the toggle.svg. See ->
https://bitbucket.org/adblockplus/spec/issues/73

[juliandoucette, 2017/09/12 22:45:38]

Acknowledged.

   mask: url(icons/trash.svg);
 }
 
 button.gear::before
 {
   -webkit-mask: url(icons/gear.svg);
   mask: url(icons/gear.svg);
 }
 
 button.gear,

[juliandoucette, 2017/09/12 22:45:38]

NIT/Suggest: button.icon

[saroyanm, 2017/09/13 15:26:37]

We already specified .icon which loads png sprite, we will use common icon
styles after refactoring.

 button.delete
 {
   border: 0px;
   padding: 0px;
   background-color: transparent;
 }
 
 button.gear::before,
 button.delete::before
 {
@@ skipping 23 matching lines @@
 }
 
 button.link
 {
   border: 0px;
   background-color: transparent;
   font-weight: 300;
   font-family: inherit;
   text-transform: none;
   padding: 0.2rem;
+}
+
+button.link:hover
+{
+  color: #5CBCE1;
 }
 
 button.list
 {
   border-style: solid;;
   border-color: #CDCDCD;
   border-width: 0px 1px 1px 1px;
   width: 100%;
   background-color: #E1F2FA;
   text-align: initial;
@@ skipping 141 matching lines @@
 }
 
 main
 {
   background-color: #FFFFFF;
   border: 1px solid #CDCDCD;
   max-width: 46.3rem;
   padding: 0px 0rem 1.4rem 0rem;
 }
 
 main > div

[juliandoucette, 2017/09/12 22:45:38]

NIT: This could use some explanation via comment (in the future)

 {
   display: none;
 }
 
 main p
 {
   margin: 0.8rem 0rem;
 }
 
 /*
   Sections
  */
 
 [role="tabpanel"] > section,
 [role="tabpanel"] > .section
 {
   padding: 2rem;
   border-top: 1px solid #CDCDCD;
 }
 
-[role="tabpanel"] > h1,

[juliandoucette, 2017/09/12 22:45:37]

Note: You would have to update these selectors accordingly if you wraped these
in a <header> like I suggested.

[saroyanm, 2017/09/13 15:26:36]

Acknowledged.

[saroyanm, 2017/09/13 16:47:59]

Done.

-[role="tabpanel"] > p
+[role="tabpanel"] > header h1,
+[role="tabpanel"] > header p
 {
   padding: 0rem 2rem;
   margin: 1.4rem 0rem;
 }
 
 section h2,
 .section h2
 {
   margin: 0rem;
 }
@@ skipping 68 matching lines @@
 #dnt
 {
   padding: 0.8rem;
   border: 1px solid #099CD0;
 }
 
 /*
   Tables
  */
 
 ul.table,

[juliandoucette, 2017/09/12 22:45:38]

Note: See related comment in new-options.html

[saroyanm, 2017/09/13 15:26:37]

See comment on the other file.

 ul.list
 {
   list-style: none;
   margin: 0rem;
   padding: 0rem;
 }
 
 .table li,
 .list li
 {
@@ skipping 29 matching lines @@
 li .display
 {
   margin: 0rem 1rem;
 }
 
 .table.list li .display
 {
   flex: 1;
 }
 
 .table.list li.empty-placeholder

[juliandoucette, 2017/09/12 22:45:36]

I wasn't able to find this in the HTML. Does it only appear when some action is
taking place?

[saroyanm, 2017/09/13 15:26:37]

We are adding the placeholder when the table is empty

 {
   padding: 1.3em 1.9em;
 }
 
 .table.list li.empty-placeholder:not(:last-of-type)
 {
   border-bottom: 0px;
 }
 
 #blocking-languages-table .display

[juliandoucette, 2017/09/12 22:45:36]

NIT: Why the name "display"?

[saroyanm, 2017/09/13 15:26:37]

We might revisit if we will only use this for filter list titles.

 {
   font-weight: 700;
 }
 
 .table.list button.link
 {
   font-weight: 700;
   text-transform: uppercase;
-}
-
-.table.list button.link:hover

[juliandoucette, 2017/09/12 22:45:37]

NIT/Suggest: Make this less specific (button.link:hover) and move it up by
a:hover

[saroyanm, 2017/09/13 15:26:36]

Acknowledged.

[saroyanm, 2017/09/13 16:47:58]

Done.

-{
-  color: #5CBCE1;
 }
 
 .table:not(.list):not(.cols) li
 {
   padding-top: 0px;
   padding-bottom: 6px;
 }
 
 .table li [data-single="visible"],
 .table li:first-of-type:last-of-type [data-single="hidden"]
@@ skipping 43 matching lines @@
 .table .col4:nth-of-type(4)
 {
   flex: 1;
 }
 
 .table.cols
 {
   font-size: 1.1rem;
 }
 
 .table.cols .display

[juliandoucette, 2017/09/12 22:45:36]

NIT: See comment above RE: display

 {
   margin: 0rem;
 }
 
 .table.cols li
 {
   padding: 0.9rem 0rem;
 }
 
 .table.cols .state
 {
   -moz-margin-start: 1.1rem;

[juliandoucette, 2017/09/12 22:45:37]

NIT: Vendor prefixes? (applies elsewhere too)

[saroyanm, 2017/09/13 15:26:38]

This probably make sense to address separately as all the browsers we are
targeting in this review supporting them, but we should include the default
implementation without prefixes if one exists.

   -webkit-margin-start: 1.1rem;
 }
 
 .table.cols .gear

[juliandoucette, 2017/09/12 22:45:37]

NIT: This doesn't seem to make a difference in Firefox or Chrome?
NIT: It's unclear why this would apply to .gear and not .delete too?

[saroyanm, 2017/09/13 15:26:37]

I think this style need to be specific to the "icon" style in general which will
be addressed after getting rid of current PNG sprite.

 {
   margin: 0rem 
 }
 
 #dialog .table.list li
 {
   display: block;
   border-width: 1px 0px 0px 0px;
   padding: 0rem;
 }
@@ skipping 27 matching lines @@
 {
   content: "";
   width: 1.3rem;
   height: 1.3rem;
   background-color: #BBB;
   -webkit-mask: url(icons/checkmark.svg);
   mask: url(icons/checkmark.svg);
   margin: 0rem;
 }
 
 #dialog .table.list li button .display

[juliandoucette, 2017/09/12 22:45:38]

NIT: See comment above about .display

 {
   flex: none;
   margin: 0rem 0.8rem;
   text-transform: none;
 }
 
 /*
   Tooltips
 */
 
 .tooltip

[juliandoucette, 2017/09/12 22:45:38]

NIT: It's confusing that you call this tooltip. It's not the tooltip itself.

[saroyanm, 2017/09/13 15:26:38]

tooltip is within the the "tooltip" container, I'll address icon accessibility
separately.

 {
   display: inline-block;
   position: relative;
   margin: 0rem;
   -moz-margin-end: 1rem;
   -webkit-margin-end: 1rem;
   
   line-height: 1.3rem;
   text-decoration: none;

[juliandoucette, 2017/09/12 22:45:37]

NIT: I wasn't able to find where text-decoration: none was necessary

[saroyanm, 2017/09/13 15:26:37]

We probably have link inside of the tooltip.

   cursor: default;

[juliandoucette, 2017/09/12 22:45:37]

NIT/Suggest: cursor: help (obviously)

[saroyanm, 2017/09/13 15:26:38]

We will revisit that separately after the style guide is updated regarding the
cursors.

 }
 
 .tooltip::before
 {
   content: "";
   width: 1.3rem;
   height: 1.3rem;
   display: block;
   background-color: #099DD1;
   -webkit-mask: url(icons/tooltip.svg);
@@ skipping 92 matching lines @@
 #all-filter-lists-table button[role="checkbox"][aria-checked="false"] + .state #state-disabled
 {
   display: inline;
 }
 
 #all-filter-lists-table
 {
   margin-bottom: 1.5rem;
 }
 
-#custom-filters-raw ~ button,

[juliandoucette, 2017/09/12 22:45:36]

NIT: Why not just #custom-filters button?

[saroyanm, 2017/09/13 15:26:37]

I'll wrap the buttons.

[saroyanm, 2017/09/13 16:47:58]

Done.

-#all-filter-lists-table ~ button
-{
-  margin: 0.8rem 0rem;
+.side-controls button
+{
+   margin: 0.8rem 0rem;
   -moz-margin-start: auto;
   -webkit-margin-start: auto;
 }
 
 #custom-filters h3

[juliandoucette, 2017/09/12 22:45:36]

NIT: It seems like this could fit into section h3, .section h3 above (or
something like it)

[saroyanm, 2017/09/13 15:26:37]

We need to target this specifically because of the table headings.

 {
   margin: 0rem;
 }
 
 #custom-filters-raw
 {
   width: 100%;
   height: 23.6rem;
   padding: 0.8rem 1.3rem; 

[juliandoucette, 2017/09/12 22:45:37]

Note: I think the difference between x and y here is a styleguide bug (TBD
separately)

[saroyanm, 2017/09/13 15:26:36]

Style guide error.

 }
 
 #custom-filters-raw:focus
 {
   outline: none;
   border: 2px solid #099CD0;
 }
 
 #custom-filters-raw-controls

[juliandoucette, 2017/09/12 22:45:37]

NIT: Looks like you're missing some margin-top here.

[saroyanm, 2017/09/13 15:26:36]

Acknowledged.

[saroyanm, 2017/09/13 16:47:58]

Done.

 {
   display: flex;
   justify-content: flex-end;
 }
 
 #custom-filters-raw-controls button
 {
   -moz-margin-start: 1rem;
   -webkit-margin-start: 1rem;
 }
@@ skipping 18 matching lines @@
   right: -1rem;
   margin-top: 1rem;
   opacity: 1;
   padding: 1.3rem;
   position: absolute;
   -webkit-transition: opacity 200ms ease-in-out 400ms,
     visibility 0ms linear 400ms;
   transition: opacity 200ms ease-in-out 400ms,
     visibility 0ms linear 400ms;
   visibility: visible;
   width: 15rem;

[juliandoucette, 2017/09/12 22:45:36]

NIT: It's not clear where this width comes from

[saroyanm, 2017/09/13 15:26:36]

eyepick

   z-index: 1;
 }
 
 html[dir="rtl"] [role="tooltip"]
 {
   right: auto;
   left: -1rem;
 }
 
 [role="tooltip"],
@@ skipping 46 matching lines @@
 {
   content: "";
   width: 0rem;
   height: 0rem;
   position: absolute;
 }
 
 [role="tooltip"]::before
 {
   border-left: 10px solid transparent;
   border-right: 10px solid transparent;

[saroyanm, 2017/09/06 19:02:57]

Note: I used this solution until we will have asset for the "tooltip arrow", see
-> https://bitbucket.org/adblockplus/spec/issues/71 

Also we have another ticket where we will finalize and improve the accessibility
of Tooltips, see -> https://issues.adblockplus.org/ticket/5542

[juliandoucette, 2017/09/12 22:45:38]

Acknowledged.

   border-bottom: 10px solid #099CD0;
   top: -10px;
   right: 15px;
 }
 
 html[dir="rtl"] [role="tooltip"]::before
 {
   right: auto;
   left: 15px;
 }
@@ skipping 23 matching lines @@
 }
 
 .context-menu li[role="menuitem"] > *
 {
   width: 100%;
   display: flex;
   border: 0rem;
   padding: 1rem 0rem;
   color: #099CD0;
   font-size: 1.1rem;
-  font-weight: 400;
   text-transform: none;
 }
 
 .context-menu li[role="menuitem"] > *:hover,
 .context-menu li[role="menuitem"] > *:focus
 {
   background-color: #E1F2FA;
   cursor: pointer;
 }
 
@@ skipping 129 matching lines @@
   background-color: #099CD0;
 }
 
 #dialog header h3
 {
   margin: 0rem;
   font-size: 1.375rem;
   font-weight: 700;
 }
 
 #dialog-close,

[juliandoucette, 2017/09/12 22:45:38]

NIT/Suggest: This could have something in common with an .icon class.

 #hide-notification
 {
   border: 0rem;
   padding: 0rem;
   margin: 0rem;
   background-color: transparent;
   cursor: pointer;
 }
 
 #dialog-close::before,

[juliandoucette, 2017/09/12 22:45:38]

NIT: Why you are adding this background image ::before the button instead of to
the button?

(The same applies elsewhere.)

[saroyanm, 2017/09/13 15:26:36]

Acknowledged. I'll address this separately.

 #hide-notification::after
 {
   content: "";
   display: block;
   height: 1rem;
   width: 1rem;
   border: 0rem;
   background-color: #FFF;
   -webkit-mask: url(icons/delete.svg);
   mask: url(icons/delete.svg);
@@ skipping 30 matching lines @@
 [data-dialog="about"] .dialog-content
 {
   text-align: center;
 }
 
 [data-dialog="about"] button
 {
   margin: 1.5rem auto 1.8rem auto;
 }
 
+[data-dialog="about"] p
+{
+  margin: 0.5rem 0rem;
+}
+
 #abp-version
 {
   margin-bottom: 2rem;
 }
 
 #dialog label
 {
   font-size: 0.9rem;
-  font-weight: 400;
   margin: 0px;
 }
 
 #dialog input[type="text"]
 {
-  border-style: solid;;

[juliandoucette, 2017/09/12 22:45:36]

NIT: The default border-style is solid? And you have two ";" here.

[saroyanm, 2017/09/13 15:26:36]

Acknowledged.

[saroyanm, 2017/09/13 16:47:58]

Done.

   border-color: #CDCDCD;
   border-width: 0px 0px 1px 0px;
   font-size: 1.3rem;
   padding: 5px;
   width: 100%;
 }
 
 #dialog .table
 {
   width: 100%;
@@ skipping 71 matching lines @@
 
 #hide-notification::after
 {
   background-color: #099DD1;
 }
 
 #hide-notification:hover::after
 {
   background-color: #5CBCE1;
 }