[RFC] Clean up of prosilver code

Note: We are moving the topics of this forum and it will be deleted at some point

Publish your own request for comments/change or patches for the next version of phpBB. Discuss the contributions and proposals of others. Upcoming releases are 3.2/Rhea and 3.3.
User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

[RFC] Clean up of prosilver code

Post by Arty »

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

User avatar
rahber
Former Team Member
Posts: 14
Joined: Mon Feb 02, 2009 10:08 am
Contact:

Re: [RFC] Clean up of prosilver code

Post by rahber »

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.
lost in area 51 :o

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Clean up of prosilver code

Post by Arty »

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
Registered User
Posts: 181
Joined: Mon Jan 24, 2005 2:10 am
Contact:

Re: [RFC] Clean up of prosilver code

Post by ecwpa »

+1


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.
Slightly better English than it was in 2005, still improving :D

User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Clean up of prosilver code

Post by brunoais »

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

<!-- IF topicrow.S_UNREAD_TOPIC --><a href="{topicrow.U_NEWEST_POST}">{NEWEST_POST_IMG}</a> <!-- ENDIF -->
If imageset wouldn't exist anymore, that code could be changed to something like this:

Code: Select all

<!-- IF topicrow.S_UNREAD_TOPIC --><a href="{topicrow.U_NEWEST_POST}" class="icon-newest">{L_NEWEST_POST}</a> <!-- ENDIF -->
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 :D

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.

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Clean up of prosilver code

Post by Arty »

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?

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Clean up of prosilver code

Post by Arty »

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.

User avatar
brunoais
Registered User
Posts: 964
Joined: Fri Dec 18, 2009 3:55 pm

Re: [RFC] Clean up of prosilver code

Post by brunoais »

Anywhere where I can see how it looks like?

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Clean up of prosilver code

Post by Arty »

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
Attachments
prosilver.zip
(276.16 KiB) Downloaded 345 times

User avatar
Arty
Former Team Member
Posts: 985
Joined: Wed Mar 06, 2002 2:36 pm
Location: Mars
Contact:

Re: [RFC] Clean up of prosilver code

Post by Arty »

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.

Post Reply