Shades of 2.0.x. :P
Ok you lot, why for you have stuff like this?
.progress_bar div.full_bar {
padding-top: 1pt;
width: 100%;
color: black;
position: absolute;
text-align: center;
font-weight: bold;
border-radius: 3px;
z-index: 2;
}
.progress_bar div.green_percent {
height: 15pt;
background-color: #c1ffc1;
background-image: linear-gradient(to bottom, #c1ffc1, green);
border-radius: 3px;
z-index: 1;
}
.progress_bar div.blue_percent {
height: 15pt;
background-color: #98b8f4;
background-image: linear-gradient(to bottom, #98b8f4, blue);
border-radius: 3px;
z-index: 1;
}
Relevant points are:
1/ Are the full_bar, green_percent and blue_percent classes used anywhere on HTML tags that are not divs? Not as far as I can see.
2/ Ok, so if not, why are you declaring them as div.full_bar, etc? Why not just use the class name? It works, it's less code, it's more efficient, and the lower degree of specificity will make customisation easier (trust me, it will).
3/ Also, while we're on the subject, are the green_percent and blue_percent classes used anywhere except inside .full_bar? Not as far as I can see, in which case there is no need to declare them as descendants. Just use the class name. That's what it's there for. Again, it works, it's less code, it's more efficient, and the lower degree of specificity will make customisation easier.
4/ Ok, so what if someone decides they don't want their "green_percent" bits green? What if they want them purple? You now have a class name which makes absolutely no sense. A fundamental principle of good markup is "use class names that are still usefully descriptive even if someone changes the presentation".
5/ Why specify z-index: 1;? Unless I'm missing something, this should not be required. It's default behaviour. Has it been tested without that declaration to see if it is actually required?
Oh goody. We haz moar. :D
/* Styles for the manage calendar section. */
dl.settings dt.small_caption {
width: 20%;
}
dl.settings dd.small_caption {
width: 79%;
}
/* Styles for the manage search section. */
dl.settings dt.large_caption {
width: 70%;
}
dl.settings dd.large_caption {
width: 29%;
}
K, so these are only used in a couple of places, so again they do not need to be specified as descendants. They do require the tag names, but only because the same class name has been used for both dd and dt. If the dd and dt were given different class names you could drop the tag names from the css.
So this is perfectly adequate with the current markup:
/* Styles for the manage calendar section. */
dt.small_caption {
width: 20%;
}
dd.small_caption {
width: 79%;
}
/* Styles for the manage search section. */
dt.large_caption {
width: 70%;
}
dd.large_caption {
width: 29%;
}
By changing the class names to make them more descriptive you could further simplify the CSS ("simplify" in the sense of using a more efficient selector).
/* Styles for the manage calendar section. */
.dt_small_caption {
width: 20%;
}
.dd_small_caption {
width: 79%;
}
/* Styles for the manage search section. */
.dt_large_caption {
width: 70%;
}
.dd_large_caption {
width: 29%;
}
Don't you mean:
/* Styles for the manage calendar section. */
.dt_small_caption {
width: 20%;
}
.dd_small_caption {
width: 79%;
}
/* Styles for the manage search section. */
.dt_large_caption {
width: 70%;
}
.dd_large_caption {
width: 29%;
}
;D
Beat you to it. :P
I should damn well hope so, CSS god :P
Anyway, we haz moar.
ul.pending_payments {
margin: 0;
padding: 0;
}
ul.pending_payments li {
list-style-type: none;
}
Y'know, ever since 2.0.x dev days there has been this in index.css:
/* A quick reset list class. */
ul.reset, ul.reset li {
padding: 0;
margin: 0;
list-style: none;
}
So, why have extra CSS for .pending payments? In fact, unless it's required for js somewhere, why have .pending_payments at all? The boring old reset class would do the same job if declared in the markup, with less CSS overall.
table.table_grid.perm_grid {
margin: 0.2em;
width: 49%;
}
Why for needz tag name? Why notz can be done with two chained classes?
Github is ----> that way >:D
Oh goody, lookit dis:
dl.admin_permissions dt {
width: 35%;
}
dl.admin_permissions dd {
width: 64%;
}
The admin_permissions class is only ever used for a dl, therefore tag name not required.
.permission_groups {
padding: 0;
margin: 0;
}
.permission_groups li {
list-style-type: none;
padding: 0.2em;
margin: 1px;
}
.perms {
width: 20px;
display: inline-block;
text-align: center;
}
/* Styles for the themes section. */
ul.theme_options {
padding: 0;
margin: 0;
}
ul.theme_options li {
list-style: none;
padding: 0.4em;
}
These ul's might as well be declared as .reset and save a pile of CSS.
Don't need tag names for the parents here:
dl.themes_list {
margin: 0;
}
dl.themes_list dt {
margin-bottom: 3px;
}
dl.themes_list dd {
font-style: italic;
white-space: nowrap;
}
Quote from: Arantor on December 28, 2014, 10:16:52 PM
Github is ----> that way >:D
Wot iz this GitHub you speak of? :P
https://github.com/SimpleMachines/SMF2.1
Oh and there shouldn't be any need to use stuff like this anywhere: margin: 0 !important;
It's not important! If someone wants a 53px margin there, they can have it. If you want 0 margin in default, just use 0 margin.
Primer: http://www.smashingmagazine.com/2010/11/02/the-important-css-declaration-how-and-when-to-use-it/
Quote from: Arantor on December 28, 2014, 10:24:18 PM
https://github.com/SimpleMachines/SMF2.1
I hate GitHub. Seriously hatez it. I am not, repeat not, doing pull requests.
The important is actually needed in several cases...
Shouldn't be. That implies a cascade balls-up somewhere.
Anyway:
/* Those collapse and expand icons are wonderful. */
.toggle_up:before, .toggle_down:before {
width: 17px;
height: 17px;
display: inline-block;
background: url(../images/toggle.png) no-repeat 0 0;
text-indent: -100em;
overflow: hidden;
content: '';
vertical-align: middle;
margin: -5px 5px 0 5px;
}
Why have a huge negative text-indent when there is no text at all in the pseudo element? :D Also, it probably doesn't need the vertical-align if using margins to correct vertical positioning. Might as well skip the align and just play with the margin.
ETA: Line height for standard body text is already declared in the body tag's font declaration. No need to repeat it here. Not really sure 1px top and bottom padding is useful here either.
/* Posts and personal messages displayed throughout the forum. */
.post, .personalmessage {
overflow: auto;
line-height: 1.4em;
padding: 1px 0;
}
Inconsistent color declarations. Better to keep them all hex, if not requiring alpha channel.
/* Calendar colors for birthdays, events and holidays */
.birthday {
color: #920ac4;
}
.event {
color: #078907;
}
.holiday > span {
color: rgb(0, 92, 255);
}
More redundant tag names:
dl.settings {
clear: right;
overflow: auto;
margin: 0 0 10px 0;
padding: 5px;
}
dl.settings dt {
width: 40%;
float: left;
margin: 0 0 10px 0;
padding: 0;
clear: both;
}
dl.settings dt.settings_title {
width: 100%;
float: none;
margin: 0 0 10px 0;
padding: 5px 0 0 0;
font-weight: bold;
clear: both;
}
dl.settings dt.windowbg {
width: 98%;
float: left;
margin: 0 0 3px 0;
padding: 0 0 5px 0;
clear: both;
}
dl.settings dd {
width: 56%;
float: right;
overflow: auto;
margin: 0 0 3px 0;
padding: 0;
}
dl.settings img {
margin: 0 10px 0 0;
vertical-align: middle;
}
/* help icons */
dl.settings dt a img {
position: relative;
vertical-align: top;
}
/* a general table class */
table.table_grid {
border-collapse: collapse;
margin: 1px 0 5px 0;
width: 100%;
}
table.table_grid td {
padding: 3px;
}
Shouldn't need to declare li here:
/* For cases where we want to spotlight something specific to an item, e.g. an amount */
.dropmenu li .amt, #top_info li .amt {
margin-left: 3px;
padding: 0 5px;
color: white;
background: #6d90ad;
border-radius: 8px;
}
.dropmenu li .active .amt, #top_info li .active .amt {
background: none;
color: inherit;
}
Come to think of it, probably don't need to declare .dropmenu there either (would need to test).
Because when I originally wrote it (which didn't use :before, I'll note), I did have it supporting text inside the element for navigational elements with textual content for speech readers.
Ah, ok. That makes sense. Probably not a bad idea to have the text and the indent. Although you could just use 100% instead. Saves a little bit of browser work since the boxes it has to draw are smaller, and the hidden overflow will still work fine.
Anyway I'll get bored soon but here's another one:
.dropmenu li a, #top_info > li > a {
padding: 0 7px 0 7px;
margin: 0;
display: block;
border: 1px solid transparent;
border-radius: 4px;
}
/* Level 1 active button. */
.dropmenu li a.active, #top_info li a.active {
background: orange;
color: #fff;
font-weight: bold;
border: 1px solid #f49a3a;
box-shadow: 0 5px 5px rgba(255,255,255,0.2) inset;
text-shadow: 1px 1px 3px rgba(0, 0, 0, 0.6);
}
Shouldn't need the li tag at all. Most likely can get away without the a tag in the second declaration as well, and just use the class name. At worst, a.active should be adequate. Zero margin shouldn't be required, offhand. Can't see why it would be.
Can't just use a.active in the latter one because that would end up targeting buttons in the button strips (e.g. the reply button in reply/add poll/mark unread) which is also a.active.
I would agree with you about going for .dropmenu a.active, #top_info a.active though.
Don't forget there are some interesting weird things in the top info area done with AJAX (which are mostly my fault)
No I meant dropping the li but keeping the .dropmenu and #top_info. Should get away with .dropmenu .active AFAICT at the moment.
The overly specific stuff in 2.0.x is a right pain sometimes. Best to keep it as low as possible.
Quote from: Antechinus on December 28, 2014, 11:07:30 PM
The overly specific stuff in 2.0.x is a right pain sometimes. Best to keep it as low as possible.
"Overly" is just the start lol. It should have rewritten from grounds up, with better html too. Not a task for the faint of heart though. :D
Oh goody. Lotsa luvverly redundant tag names here too. Might as well provide a good home for them and keep them off the streets, poor things. :D
/* The quick buttons */
div.quickbuttons_wrap {
padding: 2px 0;
width: 100%;
float: left;
}
With a bit of cunning that declaration could be ditched entirely, since .quickbuttons_wrap is just a daft leftover in two templates. It's not needed.
Ok, so:
ul.quickbuttons {
margin: 11px 0 0 0;
padding: 0 0 4px 0;
clear: right;
float: right;
text-align: right;
}
ul.quickbuttons li {
float: left;
display: inline;
margin: 0;
font-size: 0.9em;
}
ul.quickbuttons li a {
padding: 0 4px 0 5px;
display: block;
line-height: 1.9em;
float: left;
}
ul.quickbuttons li a img {
padding-bottom: 2px;
}
ul.quickbuttons li input {
margin-top: 4px;
}
ul.quickbuttons a:hover {
color: #a70;
}
.quickbuttons is always a ul, so does not that tag declared. Lose it.
Simplify .quickbuttons li a to just .quickbuttons a
Same goes for .quickbuttons li a img. Just using .quickbuttons a img should be fine.
The sprite declarations can be simplified a lot. This pile of stuff:
ul.quickbuttons li a.quote_button {
background: url(../images/quickbuttons.png) no-repeat 0 -1px;
padding: 0 2px 0 20px;
}
/* Fix for quote on reply box */
#post_modify {
border-radius: 4px;
}
ul.quickbuttons li a.modifybutton {
background: url(../images/quickbuttons.png) no-repeat 0 -59px;
padding: 0 2px 0 20px;
}
ul.quickbuttons li a.remove_button {
background: url(../images/quickbuttons.png) no-repeat 2px -29px;
padding: 0 2px 0 25px;
}
ul.quickbuttons li a.modify_button {
background: url(../images/quickbuttons.png) no-repeat 2px -59px;
}
ul.quickbuttons li a.approve_button {
background: url(../images/quickbuttons.png) no-repeat 2px -88px;
}
ul.quickbuttons li a.restore_button {
background: url(../images/quickbuttons.png) no-repeat 0 -120px;
}
ul.quickbuttons li a.split_button {
background: url(../images/quickbuttons.png) no-repeat 2px -149px;
}
ul.quickbuttons li a.reply_button {
background: url(../images/quickbuttons.png) no-repeat 0 -180px;
padding: 0px 4px 0px 22px;
}
ul.quickbuttons li a.reply_all_button {
background: url(../images/quickbuttons.png) no-repeat 0 -180px;
padding: 0px 4px 0px 22px;
}
ul.quickbuttons li a.notify_button {
background: url(../images/quickbuttons.png) no-repeat 1px -210px;
padding: 0 4px 0 22px;
}
ul.quickbuttons li a.unapprove_button {
background: url(../images/quickbuttons.png) no-repeat 2px -238px;
}
ul.quickbuttons li a.warn_button {
background: url(../images/quickbuttons.png) no-repeat 2px -270px;
}
ul.quickbuttons li.inline_mod_check {
padding: 1px 1px 1px 1px;
}
Can be boiled down to this:
/* Fix for quote on reply box */
#post_modify {
border-radius: 4px;
}
.quickbuttons a {
background-image: url(../images/quickbuttons.png);
background-repeat: no-repeat;
padding: 0 2px 0 20px;
}
.quote_button {
background-position: 0 -1px;
}
.modifybutton {
background-position: 0 -59px;
}
.remove_button {
background-position: 2px -29px;
}
.modify_button {
background-position: 2px -59px;
}
.approve_button {
background-position: 2px -88px;
}
.restore_button {
background-position: 0 -120px;
}
.split_button {
background-position: 2px -149px;
}
.reply_button {
background-position: 0 -180px;
}
.reply_all_button {
background-position: 0 -180px;
}
.notify_button {
background-position: 1px -210px;
}
.unapprove_button {
background-position: 2px -238px;
}
.warn_button {
background-position: 2px -270px;
}
.inline_mod_check {
padding: 1px;
}
Indeed. +1
Something I try to avoid in my themes - or custom code - is too much "tweaks". You know, one pixel up there etc.If something is off I rather go back, see whats causing it, use a better container/width/technique etc. Doing band-aids are so frustrating. Then again sometimes thats ALL you can do.
Yup.
And for anyone who is wondering why overly specific CSS is bad, apart from the extra bytes bloating the file, and apart from the descendant declarations just plain making it harder and more confusing to read, especially for beginners, a good example is a basic customisation someone wanted (http://www.simplemachines.org/community/index.php?topic=524638.0) earlier this year.
The default 2.0.x CSS for the buttonlist is overly specific, which meant to get his idea working Samborabora had to use the convoluted code in this post (http://www.simplemachines.org/community/index.php?topic=524638.msg3715496#msg3715496). I could see this was a mess, and knew what the problem was. So, two posts later I showed him how to rewrite the default CSS (http://www.simplemachines.org/community/index.php?topic=524638.msg3715543#msg3715543), so nothing in the default theme would be broken but customisation would be far easier.
0.o Dream... I'll eat them once my exams are over and heating my paws at home. But I must say, more I touch the CSS, i make it smaller and some people complains about how I'm limiting the usage.
Thanks a lot for spending some time on those stuff :)
Excuse me, antes... But that was uncalled for.
We did not ding you for reducing the specificity of css, we requested that a few extra classes to support mods be added into the default, even if they are not called by the curve2 standard display. There is a difference.
Quote from: Antes on December 29, 2014, 07:04:16 PM
0.o Dream... I'll eat them once my exams are over and heating my paws at home. But I must say, more I touch the CSS, i make it smaller and some people complains about how I'm limiting the usage.
Thanks a lot for spending some time on those stuff :)
Yebbut this stuff won't limit the usage. ;)
TBH I haven't tested it all yet, but most of it is a no-brainer based on basic good practice. There may be one or two places where my suggestions aren't workable. I'll see if I can get them all tested on local soon.
At some point we (Oldiesmann & I) discussed a JS file allows CSS3 elements (dunno how but) ported to IE6-8 (http://selectivizr.com/), that option still *unofficially* on table, we may bump Curve2 to be fully HTML5/CSS3 with that JS support. That surely saves lots of lines from CSS especially :first-child/:last-child elements (first_th/last_th alteration codes from files etc...).
Why even bother with IE6-8? XP is dead, and anything else will run a minimum of IE9. If someone is still insisting on running IE8, that's their problem. My 2c is it's not worth bloating the core with fallbacks for dead browsers. Waste of time, and will rapidly become less and less relevant than it already is.
The stuff concerned is mostly minor eye candy anyway, so even if someone is using a dinosaur browser it shouldn't matter too much.
don't look at me, tell that to PM/Lead Dev :P well at least we are in agreement SMF 3.0 is gonna support IE10+. I even removed windowbg2 from my theme I can do anything with CSS3 no need to use lots of stuff.
Quote from: Antechinus on December 28, 2014, 10:06:40 PM
Oh goody. We haz moar. :D
/* Styles for the manage calendar section. */
dl.settings dt.small_caption {
width: 20%;
}
dl.settings dd.small_caption {
width: 79%;
}
/* Styles for the manage search section. */
dl.settings dt.large_caption {
width: 70%;
}
dl.settings dd.large_caption {
width: 29%;
}
K, so these are only used in a couple of places, so again they do not need to be specified as descendants. They do require the tag names, but only because the same class name has been used for both dd and dt. If the dd and dt were given different class names you could drop the tag names from the css
Looks like that syntax is requited to keep proper inheritance.
K. Well Like I said I'll see if I can get them tested. The inheritance problems could be (probably are) because of over-specified stuffz in index.css borking admin.css, if you only edit the latter.
QuoteXP is deada zombie
FTFY
Quote from: Antechinus on December 29, 2014, 08:31:23 PM
K. Well Like I said I'll see if I can get them tested. The inheritance problems could be (probably are) because of over-specified stuffz in index.css borking admin.css, if you only edit the latter.
/* Lists with settings use these a lot.
------------------------------------------------------- */
dl.settings {
clear: right;
overflow: auto;
margin: 0 0 10px 0;
padding: 5px;
}
dl.settings dt {
width: 40%;
float: left;
margin: 0 0 10px 0;
padding: 0;
clear: both;
}
dl.settings dd {
width: 56%;
float: right;
overflow: auto;
margin: 0 0 3px 0;
padding: 0;
}
I wanted to drop support for everything below ie9 but the dev lead contradicted me and decided to keep supporting at least ie8. :(
There are things that I will argue vociferously with the devs about (including or not including)... but this was not worth the time to argue.
Yebbut ages ago it was decided to not worry about minor eye candy in IE8, so there's a good precedent there. ;)
yeahbut.... indeed. :D
IE8 came with Win7 and is technically supported until Win7 is also no longer supported, however the global share of IE8 is miniscule (Statcounter reports 3.18% share as of Nov 14)
Quoteuntil Win7 is also no longer supported
which is 2020
Quote from: Antechinus on December 29, 2014, 10:20:54 PM
Yebbut ages ago it was decided to not worry about minor eye candy in IE8, so there's a good precedent there. ;)
:D Indeed.
One thought is avoid the most complicated CSS at any rate, to meet even eye candy issues in older browsers. KISS does apply in CSS too. :P
If it really isn't worth our trouble to support IE8 at this point, then I'm fine with dropping support for it. I didn't realize it was barely used at this point.
I don't think you need to worry too much. IE8's CSS2.1 support is excellent, and the CSS3 stuffz in 2.1 is (AFAICT) for things that won't blow the forum to pieces if they're dropped. Things like zebra striping or gradients or rounded corners aren't that big a deal for a rapidly dying browser. The gui will still work and still look reasonable.
Please use appropriate channels (https://github.com/SimpleMachines/SMF2.1/issues/new) for requesting bug fixes/enhancements.