Skip to:
Content

Opened 8 years ago

Closed 8 years ago

Last modified 3 years ago

#3243 closed enhancement (fixed)

CSS files need commenting and organisation

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

Description

With all the good work going on for 1.3 and beyond with the default theme I think to increase the usability having some form of commenting / organisation to build on what is already there could help.

For instance an index and then listing things alphabetically / sectionally:

ie:

/* STYLE CONTENTS **************************************************************************************/
/* 1.0 activity
**************************************************************************************/
/* 2.0 adminbar **************************************************************************************/
/* >>>>>> START STYLES **************************************************************************************/
/* 1.0 activity
**************************************************************************************/
styles
/* 2.0 adminbar **************************************************************************************/
styles
/* <<<<<< END STYLES
**************************************************************************************/

I know it will be a point of discussion how these should be organised and that's a good thing. However, I think before separate files are considered this 'in one page' documentation should occur.

I feel it will help people know where to look for things along with make the CSS more accessible and maybe even encourage some new theme creators in the process.

I am willing to put forward a patch for this assuming it's something that would be wanted and agreed on a structure. I will have a think and probably update with a proposed structure assuming this is something that would be wanted to be contributed.

Attachments (5)

default.2.diff (66.0 KB) - added by karmatosed 8 years ago.
default.diff (66.0 KB) - added by karmatosed 8 years ago.
functions.diff (669 bytes) - added by karmatosed 8 years ago.
newdefault.diff (53.4 KB) - added by karmatosed 8 years ago.
defaulttoupload.diff (54.9 KB) - added by karmatosed 8 years ago.

Download all attachments as: .zip

Change History (45)

#1 @boonebgorges
8 years ago

I'm not really a front-end guy, but I will add a +1 to this proposal. bp-default should have a single stylesheet (to minimize http requests, which are already out of control with all the gravatarring), but it should be better organized with a nice table of contents.

#2 @DJPaul
8 years ago

A good example for this is the new wp-admin stylesheet; https://core.trac.wordpress.org/browser/trunk/wp-admin/css/wp-admin.dev.css

#3 @karmatosed
8 years ago

I"ll have a go at this over the week and put a suggestion up on back of looking at the wp-admin stylesheets and other standards. Won't be effecting any core code but would make a nice enhancement hopefully.

#4 @karmatosed
8 years ago

  • Owner set to karmatosed
  • Status changed from new to accepted

#5 @ezd
8 years ago

+1. This will be very helpful when styling frontend. I really like the "Tables of content" section in the top of the new wp-admin stylesheet.

#6 @DJPaul
8 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release
  • Version changed from 1.3 to 1.2

Any patches welcome

#7 @karmatosed
8 years ago

I've had a bit of a work with this and so far come up with 2 options:

*Neither is numbered/ formatted or final more gauging opinion at this point about the direction and sectioning.

  1. BuddyPress / WordPress / Generic split
BuddyPress
	Activity Stream
		Activity Posting
		Activity Listing
		Activity Comments
	Admin Bar
	Ajax Loading
	Data Tables
	Directories
	Forum
        Items 
         	Item Headers
	        Item Lists
         	Item Tabs
        	Item Body
	Messaging threads

Forms
	Buttons
        Form Elements

Navigation
        Menus
        Pagination

Structure
	Header
	Footer
	Layout
	Sidebar

Text Elements
        Headers
        Posts

WordPress
	Alignments
	Comments
	Gallery
	Images
        Lists
	Posts
  1. Less split longer contents
Activity Stream
	Activity Posting
	Activity Listing
	Activity Comments

Admin Bar

Data Tables

Directories

Forum
	
Items
	Item Headers
	Item Lists
	Item Tabs
	Item Body

Forms
	Buttons
        Form Elements

Messaging threads

Navigation
        Menus
        Pagination

Scripting
	Ajax Loading

Structure
	Header
	Footer
	Layout
	Sidebar

Text Elements
        Headers
        Posts

WordPress
	Alignments
	Comments
	Gallery
	Images
        Lists
	Posts

I would personally suggest number 1.

Once opinion is gauged I can will provide the patch for this.

Last edited 8 years ago by karmatosed (previous) (diff)

#8 @DJPaul
8 years ago

First one looks good. Go for it! :)

#9 @r-a-y
8 years ago

Mad props for taking this on!

If you're merging reset.css, make sure it's minified. No one really needs to view the reset styles with whitespace.

#10 @DJPaul
8 years ago

  • Milestone changed from Future Release to 1.3

#11 @karmatosed
8 years ago

@r-a-y: hehe thank you :)

So... had a bit of time in a corner with a stick and the CSS file.
Attached are 2 diff files.

  1. The reset.css needs to be removed under _inc/css if this is approved - sorry wasn't sure how / if to do that via diff.
  1. I tried a solution for functions.php using functions.diff leave that as a possible option not sure what want to do with that as not just a CSS file.
  1. Default.diff (do not use default2.diff) shows all the changes. Nothing should have been removed and I have done testing.

I will continue testing but suggest as such a change not just me tests this one.

Some changes from suggested format have been done as the code was explored more.

To recap the content at the top now is this with a welcome message:

/*--------------------------------------------------------------
Hello, this is the main BuddyPress Default theme file.
Please see adminbar.css in this folder for the adminbar styling.
----------------------------------------------------------------
>>> TABLE OF CONTENTS:
----------------------------------------------------------------
1.0 - Reset - Based on work by Eric Meyer
2.0 - BuddyPress
	2.1 - Activity
	     2.1.1 - Activity Listing
	     2.1.2 - Activity Comments
	2.2 - Admin Bar
	2.3 - Ajax Loading
	2.4 - Data Tables
	2.5 - Directories - Members, Groups, Blogs, Forums
	2.6 - Forum Topics
        2.7 - Items 
            2.7.1 - Item Headers
	    2.7.2 - Item Lists - Activity, Friend, Group Lists
            2.7.3 - Item Tabs
	2.8 - Private Messaging Threads
3.0 - Error / Success Messages
4.0 - Forms
	4.1 - Buttons
5.0 - Navigation
        5.1 - Pagination
6.0 - Structure
	6.1 - Content
	6.2 - Header
	6.3 - Footer
	6.4 - Sidebar
7.0 - Text Elements
        7.1 - Headers
8.0 - WordPress
	8.1 - Alignments
	8.2 - Comments
	8.3 - Gallery
	8.4 - Images
        8.5 - Lists
	8.6 - Posts
--------------------------------------------------------------*/
/*--------------------------------------------------------------
>>> START STYLES
--------------------------------------------------------------*/

This probably isn't the ultimate method to do this all I think what we have here is a good step on the road to making this file more accessible. Then we can build on this with later patches.

Last edited 8 years ago by karmatosed (previous) (diff)

@karmatosed
8 years ago

@karmatosed
8 years ago

#12 @karmatosed
8 years ago

Last edited 8 years ago by karmatosed (previous) (diff)

@karmatosed
8 years ago

#13 @djpaul
8 years ago

(In [4433]) Add table of contents to BP Default's stylesheet and reorganise file. Merge reset styles. See #3243, huge props karmatosed

#14 @DJPaul
8 years ago

  • Keywords needs-patch removed
  • Milestone changed from 1.3 to Awaiting Review

First pass is in trunk. I'm going to take this ticket out of the 1.3 milestone to keep it tidy, but any ideas or patches for further changes are welcome

#15 @r-a-y
8 years ago

I would suggest changing the order of some sections because of how CSS renders.

The main elements should be defined first like "Structure", "Text Elements" and "Forms" (you could move "Structure" to the third position if desired). These should be put directly after "Reset".

Like karmatosed says, this definitely needs thorough testing because some elements might look a little out of place afterwards.

Last edited 8 years ago by r-a-y (previous) (diff)

#16 @DJPaul
8 years ago

It's on testbp.org already for people to see if anything's broken. Looks okay so far.

#17 @hnla
8 years ago

Generally speaking the loosely excepted structure of a stylesheet runs through body/html elements to generic styles / typography through to structure or navigation then the sheet should attempt to mirror the flow (critical in some respects given the symbiotic nature of dom/css) of the markup so would start to describe the major sections of a page and it's child elements e.g head and it's descendants, content and descendants, sidebar... etc with general classes described at the end of the sheet to take advantage of the cascade.

Probably not really a practical approach with a BP layout which is difficult to conform to that structure.

I'm running through this exercise at the moment and structuring BP styles is extremely difficult so a massive nod to karmatosed for this work as I know it won't have been easy :)

Last note: when oh when will we see the back of the reset.

#18 @DJPaul
8 years ago

This needs doing for rtl.css, too.

#19 @karmatosed
8 years ago

@r-a-y: I felt the order was so open to interpretation aside from reset.css I went with alphabetical. I am happy to review other suggestions on order though.

I will probably after the rtl.css think about other orders. The good news is now it is sectioned changing the order of those sections isn't such a major task.

@hnla: For now the best is adding reset.css to the default.css.

You are right it was a bit of a juggle and I truly see this as step one in the reorganisation.

It also had to fit with some of the pre-existing comments at least so we don't remove what understanding there was before with the existing comments. I therefore kept a lot of naming as was previously - there are many changes but a lot was kept as already set to ease this.

@DJPaul: I will work this week on rtl.css assuming that doesn't need quite the 'hands off' had to do with the default.

Last edited 8 years ago by karmatosed (previous) (diff)

#20 @DJPaul
8 years ago

  • Milestone changed from Awaiting Review to 1.3

#21 @karmatosed
8 years ago

I grabbed some time tonight actually and have the following revised suggestion based on feedback.

1.0 - Reset - Based on work by Eric Meyer
2.0 - Structure
	2.1 - Content
	2.2 - Header
	2.3 - Footer
	2.4 - Sidebar
3.0 - Forms
 	3.1 - Buttons
4.0 - Text Elements
	4.1 - Headers
5.0 - Navigation
	5.1 - Pagination
6.0 - WordPress
	6.1 - Alignments
	6.2 - Comments
	6.3 - Gallery
	6.4 - Images
	6.5 - Lists
	6.6 - Posts
7.0 - BuddyPress
	7.1 - Activity
		7.1.1 - Activity Listing
		7.1.2 - Activity Comments
	7.2 - Admin Bar
	7.3 - Ajax Loading
	7.4 - Data Tables
	7.5 - Directories - Members, Groups, Blogs, Forums
	7.6 - Error / Success Messages
	7.7 - Forum Topics
	7.8 - Headers, lists and tabs - Activity, Friends, Groups
	7.9 - Private Messaging Threads

This both takes the layout 'first coming' feedback and also works to better define the ambiguous 'items' category.

7.8 - Headers, lists and tabs - better way of describing items and also can include all.

I still think keeping after the structure and so on BuddyPress and WordPres as a split makes sense. Using this basis I put WordPress before BuddyPress as it's a plugin that runs on WordPress.

I will leave this up for a few days to gauge feedback then can without too many issues restructure according to feedback.

#22 @bowromir
8 years ago

First off: This is great work Karmatosed.. very well done.. Your structure looks great and personally I think that all BuddyPress styling should be in a seperate stylesheet.. There are a couple of reasons why I think this makes sense:

1: Ease of use for 3rd party themes wanting to include BuddyPress: This is by far the biggest reason.. Currently if you'd wanted to load BuddyPress styling you'd import BP-Default.css. This means that all the global styling is also being loaded, causing massive overhead and CSS conflicts for themes who might NOT want to use the global styling BP-Default applies. Maybe you'd just want to have a very solid, up to date BuddyPress CSS starting point, and do the global styling yourself. Instead of heaving to fork BP-Default, and taking only the BP stuff, with a BP only stylesheet you could just import buddypress.css from the plugin folder. This is big step towards making BP more granular imo. For me it has never made sense to include styling for global divs like "Sidebar, Content, Container, Header" within the same CSS file. It either forced you to overwrite CSS, use !important or fork bp-default.

2: It gives the right example for WP/BP designers: Like Karmatosed says; BP is "just" a plugin. Would BBPress apply global styling in their CSS when activated?

3: Reducing HTTP request is not really an isue.. Currently style.css loads BP-Default, Adminbar.css and Reset.css. Assuming someone decided built their own BP enabled theme, they skip reset.css because they handle it themselves and import buddypress.css without the global styling. That would be a a performance gain and much cleaner css. Honestly if people concern that much about 1 http request, a CDN, Minify Plugin or even manual CSS combining can handle this request.

BP-Default is just a WordPress theme in the end.. I think it's focus should be the same as TwentyTen is for WP. A perfect example of a well coded, cleanly organised theme, that shows what you can do with WP. You can easily create Child Themes based on it, but at the same time it's full of code you can strip, re-use and learn from. With the improvements being made BP-Default can become such a theme as well, but one of the first things should be seperating BP stuff from WP/Global stuff.. Starting with the css, and hopefully in the future with the actual templates :-)

Thanks for reading :D

Last edited 8 years ago by bowromir (previous) (diff)

#23 @karmatosed
8 years ago

I think perhaps splitting is for another ticket. The point of this was to organise what we had not change it. That is purely what this ticket and the work has been about.

That's not saying it shouldn't be considered just think a new ticket then perhaps a suggestion in a patch and review of that is going to be far better to let it be considered fully. Saying it won't effect http requests is something many will want proven along with a ton of other testing such a change requests outside of a pure documentation exercise which is what this ticket was.

Last edited 8 years ago by karmatosed (previous) (diff)

#24 @hnla
8 years ago

Prefer your revision, makes more sense, 'items' is one that causes me issues when trying to work out a structure as you say mildly ambiguous terminology.

Not 100% sure on wp styles before bp ones, take the point that bp is built on wp yet bp becomes the primary display of data page to page with wp posts/pages slotting in to the bp layout so to speak, but also I think it a minor point and wouldn't really mind which way round those two were.

Funny we're both doing much the same exercise, yet I'm finding I'm approaching the sectioning in what I'm now beginning to think is a much more convoluted manner.

It occurred to me that it might make sense to contemplate the idea of perhaps running with this process a bit further. In advance of thinking about the future of bp-default and/or replacement with a new version setting up say a git hub repo and using that to very slowly build a skeletal bp framework, simply the core styles and pages, styles worked out with the correct structure as you have been demonstrating but also taking advantage of the opportunity to examine and improve the naming structure where possible and the markup, no design/graphics as such in the initial stages just the structure.

#25 @karmatosed
8 years ago

Sounds interesting and a step on from what doing here. I'll get the patch up for now though with the new format then be interested in what you want to get started hnla.

#26 @karmatosed
8 years ago

This ticket is going to stay open for a few days until people have had a chance to review and comment on the changes.

Last edited 8 years ago by karmatosed (previous) (diff)

#27 @hnla
8 years ago

Well it's probably the prime opportunity while there's no pressure to deliver to be able to gradually work up an ideal frontend package, firstly arriving at the best structure for the stylesheet(s)sans any actual rulesets, then moving to the markup and re-working that to an optimal structure with probably new-ish naming conventions, then working back to the stylesheet and starting to apply rules to that structure, if that were worked out and prepared then one can move to graphics and work up an actual presentation.

Although I generally turn my nose up at frameworks there may even be a case for looking at the 'HTML5 Boilerplate' and seeing if that could be of use?

#28 @r-a-y
8 years ago

All top-level elements should preferably be above every other child statement.

What do you think about renaming "Text Elements" to "Elements"?

"Text Elements" could be a sub-section of "Elements". Then, we can shove other elements like <pre>, <code>, <blockquote> from the Wordpress section into a sub-section called "Formatting".

Also:

  • Move all <a> styles from "Navigation" to "Elements > Formatting".
  • Move <dl>, <dt>, <dd> in 6.5 to a new sub-section called "Lists" under "Elements" or "Elements > Formatting".
  • Move <hr> from "Structure" to "Elements > Formatting"
  • Move "Headers" section directly under "Elements".
  • Move ".clear" from "Structure" to "Alignments"
  • Move "Forms" into a new sub-section in BuddyPress, as all those styles are only applicable to BP forms. The "Buttons" sub-section poses a tiny problem, as there is a little overlap between top-level and child-level statements. However, I would still be okay with moving "Forms" into "BuddyPress" though. Thoughts?

(EDIT) I've just reviewed the "Buttons" section some more and I believe it should go under "Elements" since <button> and <input> declarations are made.

Last edited 8 years ago by r-a-y (previous) (diff)

#29 follow-up: @karmatosed
8 years ago

I don't really agree with moving the a as it's navigation in sense you use to get around / to places. It's certainly not formatting. Hr is also not really a formatting thing but under something like more generic 'elements' would be ok.

.clear moving sounds fine to me.

I don't agree about moving forms as some of those styles do apply throughout. I think best remove the buttons and just have under forms as after all includes inputs so can't just put under elements.

A 'common ground' I think would be to have:

7.0 - Elements - non structure styling

7.1 - Text Elements
7.2 - Headers
7.3 - Lists (moving out from WordPress)

Under Elements hr and other core ones. I'd rather not have yet another 'empty' header and think this covers them easily without having to do another category.
Keep code, blockquote where are.

As we're running with this one today unless objections in next few hours will be submitting that patch.

Last edited 8 years ago by karmatosed (previous) (diff)

#30 @karmatosed
8 years ago

@hnla: I think we need to have a discussion in perhaps the default theme group and bash out all those ideas. I'd love to hear what you think about things and once this ticket is done maybe we can do that?

Last edited 8 years ago by karmatosed (previous) (diff)

#31 @hnla
8 years ago

@karmatosed Yep sounds good

I'd written this before the last comment was made but was blocked due to changes, some of my comments might not pertinent now.

Elements is a very generic label though it doesn't really describe anything given everything could be labeled an element so to speak. 'Text Element' likewise, is somewhat loose?

If one works to a conventional stylesheet approach it's usual to start the sheet off with all your site wide generic styles / typographic , that is heading sizes, margin defaults for primary block elements p. h# etc, anchor decs, and all the od tags like pre, blockquote, etc. I would argue that regardless of section name elements/text elements this section should really be towards the top of the sheet, under what ever this section is called I agree that 'headers' should be incorporated, the styles that are described are site wide generic, I don't think though that they need to be sub-sectioned as this is starting to over egg the pudding.

I agree on the 'buttons' aspect although think they might be better in their own section? As Buttons stand they can exist as simple text links and outside of any form element so having them under '3.0 forms' doesn't feel correct.

One thing I would comment on is to be wary of creating too complex a structure as that can start to become hard to navigate around and defeat the original intention, at least I found that to be true in running this same exercise myself with BP stylesheets.

#32 in reply to: ↑ 29 @r-a-y
8 years ago

Replying to karmatosed:

I don't really agree with moving the a as it's navigation in sense you use to get around / to places.

My main point here is that CSS order is important.

Right now, "Buttons" is above "Formatting" and the "Buttons" section declares "a.button:hover". Since the top-level a:hover is below this, a:hover's color will take precedence over what is declared for a.button:hover.

(EDIT) Brainfart! Forget I wrote that paragraph! It's late here!

Re: forms section - the declarations here use a sub-class - "standard-form", I wasn't really thinking of use-case, but of elements. As long as the "Forms" section is moved below "Elements" in the TOC, I have no beef with this! ;)

Re: <code>, <blockquote> et al. - These are not as important elements, so I'm fine with whatever works for everyone.

I would like to see hnla outline a rough TOC as well, as I'm in agreement with what he mentioned in his third paragraph above.

The more thoughts we can get from everyone the better!

Last edited 8 years ago by r-a-y (previous) (diff)

#33 @hnla
8 years ago

Right now, "Buttons" is above "Formatting" and the "Buttons" section declares "a.button:hover". Since the top-level a:hover is below this, a:hover's color will take precedence over what is declared for a.button:hover.

@r-a-y sorry to highlight that struckthrough paragraph :) but in principle this is correct, the more generic site wide applicable style would be described first e.g a:hover this sets your general required styles then later where required you would adjust this for something particular, this honours the notion of the 'Cascade' even though later styles would infer a heavier specificity through descendent selectors or classes.

Ok so in this instance things are not upset but the feel is wrong the lesser weight declaration should precede more specific styles.

At the moment I'm still knee deep in trying to fathom the structure I like and am partly hampered by the fact that I'm working things slightly arse about face as I'm actively writing styles / rulesets then jumping back and fourth re-structuring things as and when I realise something doesn't make sense which is a horrendously slow process and far from ideal also I don't want to conflict with Karmatosed's start on this which is overall working out well.

If I can make a little more headway next two days in my actual structure ( I am also working to a slightly different approach with multi sheets, separating framework structure out)I may post it for comparison.

Part of the reason I mentioned earlier the notion of using this exercise as a starting point for setting out a new set of sheets and markup files was due to there being some benefit to being able to start clean describing the ideal structure and naming conventions for elements, an exercise which I think would stand future possible bp-default themes and also a base for using with custom themes in good stead, handy to have ready to use.

#34 @karmatosed
8 years ago

Ok patch once I've double checked it will be uploaded but think for now we can wrap this up hopefully with the following structure:

1.0 - Reset - Based on work by Eric Meyer
2.0 - Structural
	2.1 - Content
	2.2 - Header
	2.3 - Footer
	2.4 - Sidebar
3.0 - Non-structural
	3.1 - Text
	3.2 - Headers
	3.3 - Lists
4.0 - Navigation
	4.1 - Pagination
5.0 - WordPress
	5.1 - Alignments
	5.2 - Comments
	5.3 - Gallery
	5.4 - Images
	5.5 - Posts
6.0 - BuddyPress
	6.1 - Activity
		6.1.1 - Activity Listing
		6.1.2 - Activity Comments
	6.2 - Admin Bar
	6.3 - Ajax Loading
	6.4 - Directories - Members, Groups, Blogs, Forums
	6.5 - Error / Success Messages
        6.6 - Forms
	6.7 - Forums, Tables and Topics
	6.8 - Headers, Lists and Tabs - Activity, Groups, Blogs, Forums
	6.9 - Private Messaging Threads	

This is not saying it can't be done later with changes. I felt the elements term just wasn't working so I clearly split structural with non-structural. As that pretty much says it all there. As a result text elements becomes text and so on.

Last edited 8 years ago by karmatosed (previous) (diff)

#35 @karmatosed
8 years ago

Diff attached: defaulttoupload.diff and ready to go.

#36 @hnla
8 years ago

I did some work in #3259 on cleaning up default, adding in missing border-radius etc one patch was committed but I spent a bit of time moving properties into order on a further patch not yet committed I hope that it can still be added or merged with this patch paul? or are the two too out of sync

#37 @DJPaul
8 years ago

The comments in the markup go from "3.2 - Headers" straight to "4.3 - Lists". Can you check the patch please?

#38 @karmatosed
8 years ago

Reuploaded the file defaulttoupload.diff sorry it was a spelling mistake of mine and nothing more.

#39 @djpaul
8 years ago

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

(In [4461]) Reorganise BP Default's stylesheet's table of contents. Fixes #3243, props karmatosed

#40 @DJPaul
3 years ago

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