New Avatar Constraint on Maintain Aspect Ratio

[Tanweth]

This may be a matter of preference, but it is an edit that I put in whosonline.php for my own website since I prefer it, and I thought I would share the code so you can see if you also prefer it. :)

The default method for maintaining aspect ratio (constraining by width) can result in skewed avatar dimensions for avatars that are significantly higher than they are wide (i.e. an unusually high avatar appears much larger than an unusually wide avatar).

Instead, I prefer to have the script check the avatar's dimensions, and constrain it by its largest dimension. So if the avatar is higher than it is wide, it will be constrained by height. If it is wider than it is high, it will be constrained by width. This avoids avatar stretching while getting closer to the benefit of constraining by both dimensions (uniform avatars).

I also changed $db->query to $db->write_query, since the MyBB Docs indicate that write_query is now preferred (http://docs.mybb.com/Database_Constants.html). Though if you have reasons I'm not aware of for using $db->query, obviously you can ignore that change.

[WildcardSearch]

I haven't tested this at all, but something initially jumps out at me: using this method we cannot ensure the width of avatars will remain constant.

Since the module works by creating table rows of avatars and computes each avatar's width based on side box width, if avatars were allowed to be of varying widths then we could overflow our side box.

As I said, I haven't proven that in testing, but I am certain it would be an issue.

I also changed $db->query to $db->write_query, since the MyBB Docs indicate that write_query is now preferred

Hehe thanks for the link. I constantly use $db->query() -- idk y I never noticed that.

[Tanweth]

Hmm, I see what you mean. I'll take another look when I get a chance and see if I can come up with a solution. My forums rarely have the opportunity of enough users online to overflow the sidebox, so I can't say that I've tested that. :P

[Tanweth]

I took another look, and as far as I can tell from the code, it shouldn't be an issue. Since the script only constrains by height if the height is longer than the width (and $avatar_height is already set to equal $avatar_width), the width of avatars that are constrained by height will always be lower than the maximum width calculated from the sidebox's width. And avatars constrained by width would be constrained in the same way that they are in Advanced Sidebox currently, so they won't exceed the maximum width. As far as I can tell, that means that a row would never overflow.

The only issue I can see is that the variable width will mean that a row of avatars will often no longer completely occupy the space provided by the row (i.e. there would be white space at the end of the row). There could even theoretically be a situation where there is room for another avatar, but that would require multiple unusually narrow avatars on the row. The white space may not be aesthetically-pleasing to some, so that may be an issue. But for me it is more aesthetically-displeasing to have high avatars towering over puny wider-than-high avatars. :P

[WildcardSearch]

Well I haven't written the idea off yet, I just thought about that when reviewing the pull request.

I am elbow-deep in the Ogre C++ library right now. Perhaps I'll make it back to PHP sometime today or tomorrow :dart:

[WildcardSearch]

Okay, I had time to test and with a small edit (two actually) this can work. And really it is an improvement so thanks in advance.

If you can change the template definition in your pull request from:

            array(
                "title" => 'asb_whosonline',
                "template" => <<<EOF
                <tr>
                    <td class="trow1">
                        <span class="smalltext">{\$lang->asb_wol_online_users} [<a href="online.php" title="Who\'s Online">Complete List</a>]<br /><strong>&raquo;</strong> {\$lang->asb_wol_online_counts}</span>
                    </td>
                </tr>
                <tr>
                    <td class="trow2">
                        <table>
                            <tr>
                                {\$onlinemembers}
                            </tr>
                        </table>
                    </td>
                </tr>
EOF
            ),

to:

            array(
                "title" => 'asb_whosonline',
                "template" => <<<EOF
                <tr>
                    <td class="trow1">
                        <span class="smalltext">{\$lang->asb_wol_online_users} [<a href="online.php" title="Who\'s Online">Complete List</a>]<br /><strong>&raquo;</strong> {\$lang->asb_wol_online_counts}</span>
                    </td>
                </tr>
                <tr>
                    <td class="trow2">
                        <table style="width: 100%; text-align: center;">
                            <tr>
                                {\$onlinemembers}
                            </tr>
                        </table>
                    </td>
                </tr>
EOF
            ),

and then update the module version to 1.4.3 (to accommodate the upgraded template) then I would be glad to merge this.

Your change causes the width to be uncertain and therefore adding style="width: 100%; text-align: center;" to the user-list table will keep everything lined up and if there is white space it will be evenly distributed.

[Tanweth]

Good idea. I just added the changes. :)

[WildcardSearch]

I tested with some really odd avatars and it really does make the table look much nicer this way.

I may even stop squishing people's avatars on my forum . . . maybe :tongue:

Seriously though, thanks for your help and support. :smiley:

[Tanweth]

Thanks for implementing it, this will make the module much more pleasant to use for me. :smile: I can't take full credit, as someone on my forums suggested the method. But I coded it myself, naturally. :tongue:

[WildcardSearch]

Ooops . . . I should've tested further. :blush:

My addition causes stretching when the avatar count is less than $settings['asb_avatar_per_row']['value'] . . .

I'll have to take a look at the best way to roll this back and get it working correctly.

[Tanweth]

LOL. I added the template edit to my own site, and everything looked fine when it was just my avatar there. Until you posted this and I had another user log on to test it. It does look rather comical I must say.

[WildcardSearch]

:laughing:

I pushed a commit that fixes it on my end. width: 100%; was the culprit.

[Tanweth]

Have you tested whether the centering is still preserved? Theoretically the text-align: center property should center the contents of the table columns, but past experience has taught me that it can be a bit dodgy as to whether CSS styles applied to the table tag are passed on to the columns.

[WildcardSearch]

It seems to be working fine but I have only tested in Firefox presently.