Skip to:
Content

Opened 8 years ago

Closed 8 years ago

Last modified 3 years ago

#2737 closed enhancement (fixed)

Default theme default.css clean up and tidy

Reported by: karmatosed Owned by: DJPaul
Milestone: 1.5 Priority: normal
Severity: Version:
Component: Templates Keywords:
Cc:

Description

The default.css has a couple of unused classes (form.standard-form is an empty class for instance) also could do with a little tidy up such as removing some white space and some alignment. It's a small thing but would be maybe a nice housekeeping thing maybe. I would be happy to provide a file if this is something wanted to happen.

Attachments (4)

default.css (34.8 KB) - added by karmatosed 8 years ago.
Formatted default.css
adminbar.css (3.7 KB) - added by karmatosed 8 years ago.
Formatted adminbar.css
adminbar.2.css (3.8 KB) - added by karmatosed 8 years ago.
default.2.css (35.9 KB) - added by karmatosed 8 years ago.

Download all attachments as: .zip

Change History (19)

#1 @DJPaul
8 years ago

Go for it!

@karmatosed
8 years ago

Formatted default.css

@karmatosed
8 years ago

Formatted adminbar.css

#2 @karmatosed
8 years ago

I've attached a default.css and adminbar.css with formatting to line up everything.

Also in default.css there is a removal of this empty class:

form.standard-form {
}

These CSS files come from trunk to make sure up to date.

#3 @hnla
8 years ago

Inconstancies in vendor prefix use has for a while puzzled me; some rulesets have the correct property declaration last in the order but many don't! why? it's sloppy :)

Nice to see proper formatting, but hazard a guess that it will be indented again, not sure where that came from but it's a pain and doesn't denote specificity or help make that aspect clearer, it feels like programmers trying to apply programming paradigms and structure that's out of keeping

#4 @boonebgorges
8 years ago

I am not a designer and don't have any strong personal feelings about CSS formatting, but I will note the following in favor of indentation:

  • http://codex.wordpress.org/CSS_Coding_Standards recommends it
  • Indentation/whitespace in PHP (like in most languages) is not semantically relevant, and is only used for readability. In that sense it makes just as much sense to impose our "structure" on CSS

One contrary consideration is CSS has to be delivered to the browser, and thus the extra bytes created by whitespace mean inefficiency. But if we minify our default stylesheets (like #1891 suggests we do for JS, and like WP does for all its stylesheets), then this issue goes away.

#5 @karmatosed
8 years ago

I'm happy to redo with indention if that's the general consensus. The indenting we had before was a little 'excessive' as we had some classes nearly half way across the page :) If we have one form of indenting fine but having a set pattern would I'd say be the best method (whatever that is).

I wasn't aware Twenty-Ten was minifying now if it is yes that does reduce the need / issue. Least then it will have one consistent look. I don't personally like that for CSS as think people use it to learn but it also has arguments for.

#6 @DJPaul
8 years ago

We're (probably) not going to minify the BP-Default theme. It'll be up to the site admin to set up appropriate cache or minify scripts. Any patch that removes indentation isn't going to get into core.

With regards to "excessive [indenting]", perhaps if you patch a very small section of the stylesheet, we can see what you have in mind.

#7 @hnla
8 years ago

Minifying isn't a look it essentially creates an unreadable file which is fine for production but obviously no use for the developer, however it - if used - clearly obviates the issues with indentation until one gets back to needing to work with the file.

@boone
It's not really a designer issue it's code and all languages tend to have syntax conventions and structures. As a developer you work with a few related languages CSS being one of them and can't really say you have "no strong feelings" :p .

If WP wants to state a particular convention that's it's prerogative and I would try not to draw huge issue on the matter but they are not in keeping with the vast majority of developers that have been working with frontend languages for a while.

- Indentation/whitespace in PHP (like in most languages) is not semantically relevant, and is only used for readability. In that sense it makes just as much sense to impose our "structure" on CSS

This is a flawed argument, yes white space is semantically null and is used for readability and that is the point readability; what does not follow is that you impose a scripting language paradigm on to a declarative language or at least a language that is significantly different in use and approach.

Common convention dictates that rulesets are left aligned and that properties are declared on separate lines this means that the human eyeball can scan quickly and easily down through the rulesets. yes there are other conventions such as declaring properties in a line but generally that is considered very awkward to read and understand let alone troubleshoot.

If we do indent - and I'll follow the consensus here but not in my own work though ;) - then can we ensure that tabs are not used but spaces or that peoples editors are set to convert tabs to spaces - tabs are an issue given that they are not an absolute measure but relative to an individuals editor settings and one can end up with indents running off to the right that become impossible to follow.

Now we can start the great Spaces VS. Tabs debate a debate that remains one of massive contention :)

#8 @hnla
8 years ago

@paul
We're (probably) not going to minify the BP-Default theme. It'll be up to the site admin to set up appropriate cache or minify scripts. Any patch that removes indentation isn't going to get into core.

Is this not even more reason not to add whitespace to the file then? If not minifying then those indents are adding bulk to the file unnecessarily ok not a huge amount but 2k can be saved in simply removing those indents

#9 @karmatosed
8 years ago

Ok for instance (hoping whitespace saves if not will upload file example:

/* > Pagination
-------------------------------------------------------------- */

div.pagination {
	margin: -20px -20px 9px -20px;
	border-bottom: 1px solid #eaeaea;
	padding: 10px 20px 10px 20px;
	color: #888;
	font-size: 11px;
	height: 16px;
}
	div.pagination#user-pag, .friends div.pagination,
	.mygroups div.pagination, .myblogs div.pagination, noscript div.pagination {
		background: #f8f8f8;
		border: none;
		padding: 8px 15px;
	}

	div.pagination .pag-count {
		float: left;
	}

	div.pagination .pagination-links {
		float: right;
	}
		div.pagination .pagination-links span,
		div.pagination .pagination-links a {
			font-size: 12px;
			padding: 0 5px;
		}
			div.pagination .pagination-links a:hover {
				font-weight: bold;
			}

div#pag-bottom {
	margin-top: 0;
}

Should be cleaned to (if keeping indent):

/* > Pagination
-------------------------------------------------------------- */
div.pagination {
	margin: -20px -20px 9px -20px;
	border-bottom: 1px solid #eaeaea;
	padding: 10px 20px 10px 20px;
	color: #888;
	font-size: 11px;
	height: 16px;
}

div.pagination#user-pag, .friends div.pagination,
.mygroups div.pagination, .myblogs div.pagination, noscript div.pagination {
	background: #f8f8f8;
	border: none;
	padding: 8px 15px;
}

div.pagination .pag-count {
	float: left;
}

div.pagination .pagination-links {
	float: right;
}
		
div.pagination .pagination-links span,
div.pagination .pagination-links a {
	font-size: 12px;
	padding: 0 5px;
}
			
div.pagination .pagination-links a:hover {
	font-weight: bold;
}

div#pag-bottom {
	margin-top: 0;
}

Or.. without indent - style sheet already attached with this in:

/* > Pagination
-------------------------------------------------------------- */
div.pagination {
margin: -20px -20px 9px -20px;
border-bottom: 1px solid #eaeaea;	
padding: 10px 20px 10px 20px;
color: #888;
font-size: 11px;
height: 16px;
}

div.pagination#user-pag, .friends div.pagination,
.mygroups div.pagination, .myblogs div.pagination, noscript div.pagination {
background: #f8f8f8;
border: none;
padding: 8px 15px;
}

div.pagination .pag-count {
float: left;
}

div.pagination .pagination-links {
float: right;
}
		
div.pagination .pagination-links span,
div.pagination .pagination-links a {
font-size: 12px;
padding: 0 5px;
}
			
div.pagination .pagination-links a:hover {
font-weight: bold;
}

div#pag-bottom {
margin-top: 0;
}

#10 @karmatosed
8 years ago

Also to add:

form.standard-form {

}

That should still be removed as seems spare unless other implications removing I'm not seeing.

#11 @nacin
8 years ago

Per WP coding standards, properties should be indented, but not entire declaration blocks. Seems like a good compromise here.

#12 @karmatosed
8 years ago

I followed the WP coding standards for blocks and selectors on new lines also made sure the sectional comments also conformed to them. Attached is a new default.css and adminbar.css with that all applied. I took from 1.3 on svn.

I also as per the WP standards converted all uppercase colours to lower case as some were lower and some upper so it's now consistent.

@karmatosed
8 years ago

@karmatosed
8 years ago

#13 @DJPaul
8 years ago

  • Owner set to DJPaul
  • Status changed from new to assigned

#14 @DJPaul
8 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In [3497] Update formatting in default theme stylesheets to match WP CSS Coding Standards. Fixes #2737, props karmatosed

#15 @DJPaul
3 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.