[RFC] Clean up of prosilver code

Arty · Post by **Arty** » Mon Jan 16, 2012 12:40 am

Introduction

There are duplicate items in css files, leftovers from IE 5/6 support that are no longer needed, leftovers from imageset -> css conversion. All of that makes code a bit messy.

Proposal

I propose to clean up prosilver files. Stuff to be cleaned up:

Duplicate color values
Duplicate entries for former imageset items that are not used by prosilver
Merge of former imageset specific items that are wrapped into link (see example #1)
Reuse class names for former imageset items that have same dimensions, like "icon newest" "icon latest"
Completely remove css tweaks
Add "phpbb" selector to root phpBB element (<body> by default), making integration of forum into custom layouts easier
Change to proper css reset instead of current * { margin: 0; padding: 0; }

More stuff might be added later.

Example #1:
From viewforum_body.html:

Code: Select all

<a href="{topicrow.U_MCP_QUEUE}">{topicrow.UNAPPROVED_IMG}</a>

which results in

Code: Select all

<a href="{topicrow.U_MCP_QUEUE}"><span class="imageset imageset-unapproved">{L_UNAPPROVED_TOPIC}</span></a>

(or something like that, class names and language variables are not correct)
That should be changed to

Code: Select all

<a href="{topicrow.U_MCP_QUEUE}" class="imageset imageset-unapproved">{L_UNAPPROVED_TOPIC}</a>

CSS Tweaks. They aren't really needed. The only browsers that require them are IE 7 and IE 8. Current tweaks.css has tweaks for IE 5.5 and IE 6 that are no longer needed. All CSS tweaks become obsolete by replacing this line in overall_header.html

Code: Select all

<html dir="{S_CONTENT_DIRECTION}" lang="{S_USER_LANG}">

with this

Code: Select all

<!--[if lt IE 8]><html dir="{S_CONTENT_DIRECTION}" lang="{S_USER_LANG}" class="ie oldie ie7"><![endif]-->
<!--[if IE 8]><html dir="{S_CONTENT_DIRECTION}" lang="{S_USER_LANG}" class="ie oldie"><![endif]-->
<!--[if gt IE 8]><html dir="{S_CONTENT_DIRECTION}" lang="{S_USER_LANG}" class="ie"><![endif]-->
<!--[if !(IE)]><!--><html dir="{S_CONTENT_DIRECTION}" lang="{S_USER_LANG}"><!--<![endif]-->

and instead of fishy code use selectors:

Code: Select all

.whatever { stuff for all browsers }
.ie .whatever { stuff for all IE 9 and older }
.oldie .whatever { stuff for IE 8 and older }
.ie7 .whatever { stuff for IE 7 }

If that change is applied, tweaks.css can be removed and changes merged into other css files using .oldie and .ie7 selectors.

Pros

Code will be much cleaner.

Cons

It breaks compatibility with 3.0. I think its not a problem, with number of changes applied to prosilver, it will be easier to redo style from 3.1's prosilver than to apply code changes from 3.0. to 3.1, making compatibility point moot.

Notes

I suggest to submit several patches for different parts of code clean up: color values in one patch, imageset leftovers in other patch, outdated css tweaks in third patch, etc.

Tickets and patches

CSS Reset: ticket, pull request - merged
Removing duplicate colors: ticket, pull request - merged
Updating CSS tweaks: ticket, pull request - merged

rahber · Post by **rahber** » Mon Jan 16, 2012 4:27 pm

i would also like to suggest that we use base div reference in css that would make phpBB integration easy.
As prosilver will be the main base line for creating diffrent styles so if we have base line people could integrate other files and headers / menus easily without the problem of duplicates css entries.
for example

.navbar should be changed to #phpbb .navbar.

Arty · Post by **Arty** » Mon Jan 16, 2012 4:48 pm

Good idea, but instead of id I suggest to use class. This way if forum content is split into several parts, all parts could use phpbb styling, like this

Code: Select all

<body>
<header> custom header here </header>
<section class="phpbb section-{SCRIPT_NAME}"> phpbb content here </section>
<section> more custom stuff here </section>
<footer class="phpbb section-{SCRIPT_NAME}"> phpbb footer here </footer>

And on that note, added Rahber's idea + one more point to list: add correct css reset.

ecwpa · Post by **ecwpa** » Mon Jan 16, 2012 5:12 pm

+1

Arty wrote: - Duplicate color values

There was any reason for doing this? Why content.css has colors defined to begin with?

Having colours.css is a great idea, but sectioning parts of the same element it's not.

Maybe we need a new convention for css files.

brunoais · Post by **brunoais** » Thu Feb 02, 2012 11:21 am

Copied here as Arty requested.

Arty wrote: Currently <img> tags are used in code for last post and newest post icons, like this:
Code: Select all
<a href="{topicrow.U_NEWEST_POST}">{NEWEST_POST_IMG}</a> 
If imageset wouldn't exist anymore, that code could be changed to something like this:
Code: Select all
<a href="{topicrow.U_NEWEST_POST}" class="icon-newest">{L_NEWEST_POST}</a> 
and would require adding something like this to css:
Code: Select all
.icon-newest { 
    display: inline-block;
    background: url(newest.png) 0 50% no-repeat;
    width: 0; /* padding is used for width */
    height: 10px; /* image height */
    padding-left: 11px; /* offset text to hide it, equal to image width */
    overflow: hidden; /* hide text */
}
That would get rid of <img> tag and put meaningful text instead of it. After applying that css it would look the same as it used to.

Just one detail. Instead of using classes like @class="icon-newest", I believe we should use classes like @class="icon newest". Reason?
We, for any reason, may want to apply some specific style to all icons no matter which they are. Instead of repeating the styling for each style or use a huge list of ',,,,,' for each icon, we can just use the selector ".icon".
I don't know if this applies but imagine that you want all icons to be inline-blocks with overflow hidden. You may do something like this:

Code: Select all

.icon { 
    display: inline-block;
    overflow: hidden; /* hide text */
}

Or you could apply these two lines of code to every single icon. How may icons are there? Some, right? That's how many. Also, if we want to change this to all icons how do we do it? I think you got the picture. Please note that you can, however, create exceptions (by following the CSS hierarchy rules)

Code: Select all

.icon { 
    display: inline-block;
    overflow: hidden; /* hide text */
}
.icon.newest { 
    display: block;
}

This will make all icons inline-blocks with overflow hidden except the icon newest which is a block and not an inline-block.

Extra: With this we may be able to save some chunks of CSS code and bandwith

Here I'm talking about icons but this applies to anything we create with this way of thinking but that is away from the purpose of this RFC.

Arty · Post by **Arty** » Fri Feb 03, 2012 3:04 pm

I'm having second thoughts about this bit:

Arty wrote:- Add "phpbb" selector to root phpBB element (<body> by default), making integration of forum into custom layouts easier

It would require adding .phpbb to each entry in style sheet, which means there will be merge conflicts every time there are changes in 3.0 css files and it will affect all existing pull requests that edit css files.

Maybe postpone this until 4.0?

Arty · Post by **Arty** » Fri Feb 03, 2012 11:14 pm

Resetting CSS

Ticket: http://tracker.phpbb.com/browse/PHPBB3-10617
Patch: https://github.com/phpbb/phpbb3/pull/553

Added proper CSS reset. Style looks almost exactly the same as it used to, but with HTML5 support for old browsers and it is easier to integrate into layouts that also use CSS reset, such as almost any WordPress theme.

brunoais · Post by **brunoais** » Fri Feb 03, 2012 11:30 pm

Anywhere where I can see how it looks like?

Arty · Post by **Arty** » Fri Feb 03, 2012 11:35 pm

Sure. Try this prosilver on your 3.1 installation

Or you can check out that repo from within your local phpbb3 repository (and do the same for other forks, allowing you to work with branches from other people's forks):

Code: Select all

git remote add ca git://github.com/cyberalien/phpbb3.git
git fetch ca
git checkout ca/feature/prosilver-cleanup/css-reset-v2

Arty · Post by **Arty** » Sat Feb 04, 2012 9:17 am

Removing duplicate colors

Ticket: http://tracker.phpbb.com/browse/PHPBB3-10619
Patch: https://github.com/phpbb/phpbb3/pull/556

Removed color values from all css files other than colours.css. In shortcut declarations where color cannot be removed without splitting whole declaration, such as border and background I've replaced color value with "transparent" that is overwritten in colours.css with correct color value.

Didn't modify print.css because it is no linked to other css files.

Development Discussion Board

[RFC] Clean up of prosilver code

[RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code

Re: [RFC] Clean up of prosilver code