Inconsistent markup in Profile.template.php

[Antechinus]

Have not checked everywhere for other instances of this yet (will do that too) but...

Code (Profile.template.php) Select Expand

<div class="cat_bar">
<h3 class="catbg">
Attachments - Antechinus
</h3>
</div>
<div class="floatleft">
<div class="pagesection"></div>
</div>
<table class="table_grid" id="attachments" style="width:100%">
You cannot view this attachment.

The problem here is that it is different to the standard message index, display, unread, etc markup for the pages. The difference creates problems when trying to style the pages links, as I have just found out when playing with variants for this site.

Those other (standard) pages are like this...

Code Select Expand

<div id="content_section">
<div id="main_content_section">
<div id="recent" class="main_content">
<div class="pagesection">
<div class="pagelinks floatleft">
<a href="#bot" class="button">Go Down</a>
<span class="pages">Pages</span><span class="current_page">1</span>
</div>
<div class="buttonlist floatright"></div>
</div>
<div id="unread">
<div id="topic_header" class="title_bar">
You cannot view this attachment.

Themers are going to be irritated about this. It screws up styling rather dramatically. It would be far better to make all templates consistent, with the .pagesection parent div NOT being wrapped inside a .floatleft div.

Will check for other examples of the current misdemeanour.

ETA: To clarify, the core issue is that on the borked templates .pagesection is wrapped inside div.floatleft. Which should not be necessary (because if it is, something else is screwed).

[Antechinus]

Aha. Just realised the borked markup is also missing the .pagelinks class. So it's a "double bug". Will run a thorough search and find all instances.

[Antechinus]

If anyone else is playing with page index styling and wants a workaround, this makes presentation of the "dodgy" pages consistent with the "normal" pages...

Code Select Expand

/* Page index presentation. */
.pagelinks {
    display: flex !important;
    flex-wrap: wrap;
    padding: 0 1px;
    color: transparent;
}
/* Odd cases with dodgy markup. */
.cat_bar + .floatleft > .pagesection,
.table_grid + .pagesection > .floatleft:first-child {
    display: flex !important;
    flex-wrap: wrap;
    padding: 4px 1px;
    color: transparent;
}
This is assuming there aren't more variations on the dodgy ones that I haven't found yet. Famous last words.

[Antechinus]

Yay! Found another variation on the same muppetry! Even better, this one uses the same parent element id as the unread pages (which have normal markup for the page index) so trying to target by parent element will screw everything. If it's worth borking, it's worth borking properly.

Code Select Expand

<div id="recent" class="main_section">
        <div class="cat_bar">
            <h3 class="catbg">
                <span class="xx"></span>Recent Posts
            </h3>
        </div>
        <div class="pagesection">
             <span class="pages">Pages</span>
            <a class="nav_page" href="https://www.simplemachines.org/community/index.php?action=recent;start=30"></a>
            <a class="nav_page" href="https://www.simplemachines.org/community/index.php?action=recent;start=0">1</a>
            <span class="expand_pages"> ... </span>
            <a class="nav_page" href="https://www.simplemachines.org/community/index.php?action=recent;start=20">3</a>
            <a class="nav_page" href="https://www.simplemachines.org/community/index.php?action=recent;start=30">4</a>
            <span class="current_page">5</span>
            <a class="nav_page" href="https://www.simplemachines.org/community/index.php?action=recent;start=50"><span class="main_icons next_page"></span></a>
        </div>
        <div class="windowbg"></div><!-- $post[css_class] -->The problem is on action=recent, meaning the acual recent posts pages, not the unread pages. It would probably be possible to work around it with more convoluted CSS, but this is getting silly. Consistent markup would be a far better solution.

[Antechinus]

Have made a complete list of all templates that call this evil. I'll go through and check all instances, to see how consistent it can be made without too much work.

Display.template.php - 2 instances - Lines 233 and 270
Errors.template.php - 2 instances - Lines 74 and 183
Help.template.php - 1 instance - Line 123
ManageMembergroups.template.php - 2 instances - Lines 556 and 665
Memberlist.template.php - 2 instances - Lines 24 and 130
MessageIndex.template.php - 2 instances - Lines 79 and 316
ModerationCenter.template.php - 3 instances - Lines 345, 387, and 447
MoveTopic.template.php - 2 instances - Lines 235 and 251
PersonalMessage.template.php - 6 instances - Lines 236, 692, 900, 949, 1928, and 1975
Profile.template.php - 4 instances - Lines 498, 551, 664, and 714
Recent.template.php - 6 instances - Lines 27, 52, 80, 184, 231, and 329
ReportedContent.template.php - 3 instances - Lines 76, 393, and 428
Search.template.php - 4 instances - Lines 320, 401, 443, and 473
Who.template.php - 2 instances - Lines 29 and 111

[Antechinus]

Went through these and had a look at them. There are only 8 files with anomalous markup, and making them all consistent should be very easy. In some cases it is as simple as adding the .pagelinks class to an existing div. Since GitHub has apparently taken the drastic step of actually attempting to make their system usable for average humans*, I'll see if I can get a PR together.

Code Select Expand

Errors.template.php
[2 hits] Line 73
Line 182

Help.template.php
[1 hit] Line 125

ManageMembergroups.template.php
[2 hits] Line 596
Line 665

ModerationCenter.template.php
[3 hits] Line 344
Line 387
Line 446

MoveTopic.template.php
[2 hits] Line 235
Line 251

PersonalMessage.template.php
[6 hits] Line 236
Line 692
Line 900
Line 949
Line 1928
Line 1975

Recent.template.php
[6 hits] Line 27
Line 52

Search.template.php
[4 hits] Line 320
Line 401
Line 443
Line 473
*Took them long enough. Linux geeks.

[Antechinus]

While I think of it, there is another inconsistency related to page index markup. Most templates that include the .pagesection class* do not have any way of distinguishing between the upper and lower instances. However, some templates do. Who.template.php has this:

Code Select Expand

<div id="mlist">
<div class="pagesection">
<div class="pagelinks floatleft"></div>
<div class="selectbox floatright" id="upper_show"></div>
</div>
<table class="table_grid">
</table>
<div class="pagesection" id="lower_pagesection">
<div class="pagelinks floatleft" id="lower_pagelinks"></div>
<div class="selectbox floatright"></div>
</div><!-- #lower_pagesection -->
</div><!-- #mlist -->
That's quite handy, in that it gives you easy different styling options for upper and lower instances of both .pagesection and .selectbox. In practice I doubt different styling options will be needed for upper and lower instances of .selectbox, or for upper and lower instances of .pagelinks, but the ID on one of each does no harm.

The case with .pagesection is different because that is the wrapper for the page index and .selectbox. Depending on the presentation you are after, and depending on which template you are dealing with, you may want to handle the upper and lower instances of .pagesection differently. This can sometimes be done by getting a bit convoluted with CSS (:first-child, adjacent selectors, body class from action, etc) but since some templates already make it very easy, it seems like a good idea to make all of them very easy.

But, the other template that does make it easy doesn't do it the same way as Who.template.php.
For example, Display.template.php has this:

Code Select Expand

<div class="pagesection top">
<div class="buttonlist floatright"></div>
<div class="pagelinks floatleft"></div>
<div class="mobile_buttons floatright"></div>
</div>
<div id="forumposts"></div><!-- #forumposts -->
<div class="pagesection">
<div class="buttonlist floatright"></div>
<div class="pagelinks floatleft"></div>
<div class="mobile_buttons floatright"></div>
</div>

So in this case, instead of an ID on the lower instance of .pagesection, there is a class on the upper instance. Which seems a bit nuts. Classes are generally less likely to cause issues than ID's, so going with a class is probably the more sensible option. The best way of handling it may be to do all of them like this:

Code Select Expand

<div class="pagesection">
<div class="buttonlist floatright"></div>
<div class="pagelinks floatleft"></div>
<div class="mobile_buttons floatright"></div>
</div>
<div id="forumposts"></div><!-- #forumposts -->
<div class="lower_pagesection">
<div class="buttonlist floatright"></div>
<div class="pagelinks floatleft"></div>
<div class="mobile_buttons floatright"></div>
</div>

That means for common styling your CSS becomes this:

Code Select Expand

.pagesection, .lower_pagesection {
/* Lurid eye candy goes here. */
}
And if you want them different you use this:

Code Select Expand

.pagesection {
/* Lurid eye candy goes here. */
}
.lower_pagesection {
/* Extra horrors for added gruesome. */
}
*Not all of them, at the moment, because inconsistency y'know.