[RFC|Merged] Removal of imagesets

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Post Reply
User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Berlin, Germany
Contact:

Re: Removal of imagesets

Post by naderman »

CyberAlien wrote:
naderman wrote:Are there any legitimate use cases for non-theme images in the template? Cases where one would use <img> and want to have an image in the template?
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.
I understand that. What I was asking is: Are there cases where <img> is used directly in the template and not coming out of a variable? And are there use cases where this CSS solution would not work? In particular in non-prosilver/subsilver2 styles, meaning do other styles make use of img tags in other ways?

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

Re: Removal of imagesets

Post by Arty »

Only logo is used that way

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

Re: Removal of imagesets

Post by Arty »

It is definitely doable. Here is a commit for yesterday's version of phpBB 3.1: https://github.com/cyberalien/phpbb3/co ... 085b31d708

I've changed only index, viewforum and viewtopic pages so far and only for prosilver. I've hardcoded logo into overall_header.html, but without width/height. The rest of images are in css files.

All language specific buttons have fallback code in main css files, pointing to English version. So if you use a different language that doesn't have a language pack, English images would be shown.

Style doesn't look exactly the same; there are minor differences:
Image
First block is from old style, second block from modified style. Image is aligned differently because in first block its <img> tag, in second block its a background image added to link. Same difference applies to "reported post" block inside post.

User avatar
Mighty Gorgon
Registered User
Posts: 14
Joined: Wed Sep 17, 2003 8:52 am
Location: Italy
Contact:

Re: Removal of imagesets

Post by Mighty Gorgon »

I think is a good idea to remove imagesets folders and move those images to CSS instead of IMG tags, I can't find now any extra good reasons apart of those already listed in the previous posts.

The only issue is that old styles would not be easily/quickly upgraded to phpBB 3.1 (as naderman suggests) and users could complain about that.

topdown
Registered User
Posts: 12
Joined: Sat Dec 01, 2007 5:04 am

Re: Removal of imagesets

Post by topdown »

You have my vote. ++1
I would love the possibility to sprite all of these icons as they should be.

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

Re: Removal of imagesets

Post by Arty »

Done!

Commit that includes all changes: https://github.com/cyberalien/phpbb3/co ... 61b7cf1808

Notes:
- In database_update for schema changes I've added new key: drop_table, but it isn't implemented
- style.php still exists and is working correctly, but it is due to be removed per [RFC|Accepted] No db storage of stylesheets and templates
- I didn't change folder/topic images variables structure as it was written in first post

If you'll be manually updating from another 3.1 version, drop column theme_id from styles table, both imageset tables and remove imageset module from acp.

How to make RFC out of this?

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Berlin, Germany
Contact:

Re: Removal of imagesets

Post by naderman »

Probably just update the first post of this topic to include all the answers to questions that were brought up, so it explains the proposal completely, and then we'll move this into an RFC topic.

User avatar
naderman
Consultant
Posts: 1727
Joined: Sun Jan 11, 2004 2:11 am
Location: Berlin, Germany
Contact:

Re: Removal of imagesets

Post by naderman »

Looking at your commits, you now have:

https://github.com/cyberalien/phpbb3/co ... 085b31d708 (original commit)
https://github.com/cyberalien/phpbb3/co ... faf655bcea (revert of original commit)
https://github.com/cyberalien/phpbb3/co ... 61b7cf1808 (actual commit)

Please get rid of the first and second commits entirely (ask on IRC if you need help with doing that).

Please also consider, that making one large commit for a lot of things is a bad idea. If we later find a bug and track it down to this commit, it will be difficult to identify what exactly caused the bug, because so many places were changes at once in this commit. However in this particular case I guess it's ok to remove everything relating to imagesets in a single commit. Just please don't squash commits together unecessarily.

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

Re: Removal of imagesets

Post by Arty »

First post edited. Is it ok?

Commits: sure, I'll ask on irc how to get rid of it. If you want to, I can split it into several commits that affect only certain parts of phpBB, such as php files, installer/upgrader, subsilver2, prosilver.

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

Re: Removal of imagesets

Post by Arty »

Split into 5 commits: https://github.com/cyberalien/phpbb3/co ... cket/10336

Huge thanks for guiding me through git!!!

Post Reply