Skip to content

Loading…

Inline CSS → embedded CSS (except `display: none`) #1172

Open
wants to merge 2 commits into from

2 participants

@Zearin

If this works for you, I can do more. :)

Thanks for uBlock!

@Zearin

Values in attribute selectors should be quoted: [data-i18n="3pExternalListsHint"]

I’d forgotten that! Will fix shortly.

(Also, that's a digit 1 in the markup, not a lowercase L: i18n is a common abbreviation for "internationalization" because there are 18 characters between i and n; similarly, l10n means "localization")

Ah okay. That makes more sense. I thought it was “l8n” for “localization” and “i8n” for “internationalization” because the “8” sounds kind of like the “-za-” part of both words. (I thought it was kinda dumb, but that’s what I figured. Looks like I turned out to be the dumb one! :p)

Will fix this shortly, too.

@Zearin

(Sorry this took me a while to get back to!)

The issues have been fixed, and I’ve rebased my changes onto your latest master.

@Gitoffthelawn

Hard-coded font sizes do not work for accessibility purposes. Users who use larger default font sizes will not have properly sized fonts. In CSS, percentages and values such as "small" and "large" allow accessible use.

@Zearin

Hard-coded font sizes do not work for accessibility purposes. Users who use larger default font sizes will not have properly sized fonts. In CSS, percentages and values such as "small" and "large" allow accessible use.

Oh, I strongly agree! I actually intended to propose this in a future Pull Request. The only reason I didn’t is because I wanted my first PR to this project to stay small and focused on one thing: making inline styles embedded (except for display: none, which I assume interacts with controller scripts, so I didn’t want to potentially break those).

I will gladly update the font sizes shortly!

@Zearin

@Gitoffthelawn Okay, 13px is now small.

After this PR is done, I can continue making more small updates, if you wish.

@Gitoffthelawn

AFAIK, uBlock is open to the whole world to make contributions!

BTW, why is it better to make the styles embedded?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 20, 2015
  1. @Zearin
Commits on Aug 21, 2015
  1. @Zearin
This page is out of date. Refresh to see the latest.
Showing with 14 additions and 3 deletions.
  1. +10 −2 src/3p-filters.html
  2. +4 −1 src/settings.html
View
12 src/3p-filters.html
@@ -6,6 +6,14 @@
<link rel="stylesheet" type="text/css" href="css/common.css">
<link rel="stylesheet" type="text/css" href="css/dashboard-common.css">
<link rel="stylesheet" type="text/css" href="css/3p-filters.css">
+<style>
+#externalListsDiv > p {
+ margin: 0.25em 0 0 0
+ }
+#externalListsDiv > p[data-i18n="3pExternalListsHint"] {
+ font-size: small;
+ }
+</style>
</head>
<body>
@@ -48,9 +56,9 @@
</li>
</ul>
<div id="externalListsDiv">
- <p data-i18n="3pExternalListsHint" style="margin: 0 0 0.25em 0; font-size: 13px;"></p>
+ <p data-i18n="3pExternalListsHint"></p>
<textarea id="externalLists" dir="ltr" spellcheck="false"></textarea>
- <p style="margin: 0.25em 0 0 0"><button id="externalListsApply" disabled="true" data-i18n="3pExternalListsApply"></button></p>
+ <p><button id="externalListsApply" disabled="true" data-i18n="3pExternalListsApply"></button></p>
</div>
</div>
View
5 src/settings.html
@@ -22,6 +22,9 @@
ul#userSettings {
list-style-type: none;
}
+#localData {
+ border-top: 1px solid #ccc
+ }
#localData > ul > li {
margin-top: 1em;
}
@@ -45,7 +48,7 @@
<!-- <li><input id="experimental-enabled" type="checkbox" disabled><label data-i18n="settingsExperimentalPrompt" for="experimental-enabled"></label> -->
</ul>
-<div id="localData" style="border-top: 1px solid #ccc">
+<div id="localData">
<ul>
<li>
<li style="display: none;"><span data-i18n="settingsLastBackupPrompt"></span><ul>
Something went wrong with that request. Please try again.