Issue 2410 - Improved accessibility of dialogs in options page

skin/options.css

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2015 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 461 matching lines @@
 .tabs.horizontal li.active
 {
   border-bottom: 2px solid #1E8728;
   color: black;
   font-weight: 600;
   padding-bottom: 10px;
 }
 
 .icon, .table input[type="checkbox"]::before, .table button.delete,
 #content-help a::before, #dialog-close::before,
-#custom-filters-add button::after
+#custom-filters-add button::after,
+#dialog-body button::before

[saroyanm, 2015/10/01 16:31:40]

Nit: should not exceed the 80 char limit if you put inline with the line before.

[saroyanm, 2015/10/01 17:27:04]

Just noticed that the consistency in either case is missing for selectors, feel
free to ignore my comments regarding that.

 {
   background-image: url(options-sprite.png);
   display: inline-block;
 }
 
 .icon-add
 {
   background-position: -0px -0px;
   cursor: pointer;
   height: 18px;
   min-width: 18px;
 }
 
 .icon-update
 {
   background-position: -34px -0px;
   cursor: pointer;
   height: 18px;
   width: 18px;
 }
 
 .icon-edit
 {
   background-position: -17px -0px;
   cursor: pointer;
   height: 18px;
   width: 18px;
 }
 
-.icon-arrow,
-#content-help a::before
+#content-help a::before,
+#dialog-body button::before

[saroyanm, 2015/10/01 16:31:41]

Nit: should not exceed the 80 char limit if you put inline with the line before.

[Thomas Greiner, 2015/10/02 10:27:29]

Sure but it's more readable if each selector is on its own line, don't you think
so?

[saroyanm, 2015/10/02 13:18:50]

I agree with you, from readability point of view having every selector on it's
own line make more sense, but it makes file also bigger, so I will vote for
current approach if we are okey having bigger files.

[Thomas Greiner, 2015/10/02 14:17:32]

Bigger files should not be an issue as long as the code inside is well
structured and easily understandable.

 {
   background-position: 0px -42px;
   content: "";
   cursor: pointer;
   height: 11px;
   vertical-align: middle;
   width: 7px;
+  -moz-margin-end: 12px;

[saroyanm, 2015/10/01 16:31:40]

Just as remark: I think from some point we should also use standardized version
as well:
margin-inline-end (same for other similar elements).

[Thomas Greiner, 2015/10/02 10:27:29]

Unfortunately, there's no standardized version yet. All there is right now is a
draft so at this point it's not stable enough to rely on a non-prefixed version.

[saroyanm, 2015/10/02 13:18:50]

I see, make sense also not use border-inline-start-width in first run page, I
think we used that property there, that why I thought it standardized. 

[Thomas Greiner, 2015/10/02 14:17:32]

I guess we can tackle that whenever we make changes to firstRun.css

+  -webkit-margin-end: 12px;
 }
 
 .controls
 {
   border-top: 1px solid #CDCDCD;
   padding-top: 8px;
   -moz-padding-start: 16px;
   -webkit-padding-start: 16px;
   position: relative;
 }
@@ skipping 462 matching lines @@
   vertical-align: middle;
   -moz-margin-end: 6px;
   -webkit-margin-end: 6px;
 }
 
 #dialog #dialog-body
 {
   margin: 4px 24px;
 }
 
-#dialog h3
+#dialog h3,
+#dialog label

[saroyanm, 2015/10/01 16:31:40]

Nit: should not exceed the 80 char limit if you put inline with the line before.

 {
   font-size: 14px;
+  font-weight: 600;
   margin: 0px;
 }
 
 #dialog input[type="text"],
 #dialog input[type="search"]
 {
   font-size: 16px;
   margin-top: 10px;
   padding: 5px;
   width: 100%;
@@ skipping 22 matching lines @@
   flex: 1;
   font-size: 16px;
   color: #FFFFFF;
 }
 
 #dialog-body .dialog-content-block
 {
   padding: 12px 0px;
 }
 
-#dialog .button-wrapper
+#dialog-body button
 {
   background-color: #F5F5F5;
+  border: none;
+  color: #3A7BA6;
+  cursor: pointer;
+  display: block;
+  font-family: inherit;
   margin-top: 8px;
   padding: 10px 16px;
-  cursor: pointer;
-  width: auto;
-}
-
-#dialog .button-wrapper span:not(.icon)
-{
-  color: #3A7BA6;
-  display: inline-block;
-  -moz-margin-start: 15px;
-  -webkit-margin-start: 15px;
-  vertical-align: top;
+  text-align: left;

[saroyanm, 2015/10/01 16:31:40]

Please also consider the case for RTL.

[Thomas Greiner, 2015/10/02 10:27:29]

Done.

+  width: 100%;
 }
 
 #dialog .url
 {
   margin-top: 10px;
   margin-bottom: 20px;
   word-wrap: break-word;
 }
 
 body:not([data-dialog="custom"]) #dialog-title-custom,
 body:not([data-dialog="custom"]) #dialog-content-custom,
 body:not([data-dialog="language"]) #dialog-title-language,
 body:not([data-dialog="language"]) #dialog-content-language,
 body:not([data-dialog="predefined"]) #dialog-title-predefined,
 body:not([data-dialog="predefined"]) #dialog-content-predefined,
 body:not([data-dialog]) #dialog
 {
   display: none;
 }
 
 #other-language .button-add
 {
+  background-color: transparent;
+  margin: 0px;
+  padding: 0px;
+  width: auto;
   -webkit-border-end: 1px solid #CDCDCD;
   -moz-border-end: 1px solid #CDCDCD;
   -webkit-padding-end: 10px;
   -moz-padding-end: 10px;
   -webkit-padding-start: 0px;
   -moz-padding-start: 0px;
 }
 
+#other-language .button-add::before
+{
+  display: none;
+}
+
 #other-language .display
 {
   -webkit-margin-start: 10px;
   -moz-margin-start: 10px;
 }