Page MenuHomePhabricator

Cleanup clearfix CSS technical debt in SocialProfile
Open, Needs TriagePublic

Description

Currently, SocialProfile provides a clearfix.css file which is a shared stylesheet that might be used by other social tools extensions. Let's break this file down into sections.

Line 1 - 3:

shared/clearfix.css
.cleared {
	clear: both;
}

This is code duplication and thus technical debt, since MediaWiki already provides a CSS class, .visualClear.


shared/clearfix.css
.clearfix:after {
	content: ".";
	display: block;
	height: 0;
	clear: both;
	visibility: hidden;
}

.clearfix { display: inline-block; }

Just extend the visualClear class with a subclass. We can write something like this below. visualClear will already contain the statement clear: both;, so there's no need to write it in .clearFix since it's already been applied.

<div class="visualClear clearfix"></div>
shared/clearfix.css
.clearfix {
	display: inline-block; 
}

.clearfix:after {
	content: ".";
	display: block;
	height: 0;
	visibility: hidden;
}

shared/clearfix.css
/* Monaco is different */
.skin-monaco .clearfix { display: block; }

From my understanding, this needs to live under the Monaco skin.

Event Timeline

Running git grep --text cleared through the social tools extensions only seemed to give one result which was in BlogPage:

BlogPageClass.php:              $output->addHTML( '<div class="cleared"></div>' . "\n" );
BlogPageClass.php:                                              <div class=\"cleared\"></div>
BlogPageClass.php:                              <div class="cleared"></div>';
BlogPageClass.php:                              <div class="cleared"></div>';

I was actually expecting more results, so I'll run through them again just in case.

Change 334804 had a related patch set uploaded (by SamanthaNguyen):
v 2.4.6 - Replace custom class with core-provided class

https://gerrit.wikimedia.org/r/334804

This is code duplication and thus technical debt, since MediaWiki already provides a CSS class, .visualClear.

That's correct.

shared/clearfix.css
/* Monaco is different */
.skin-monaco .clearfix { display: block; }

From my understanding, this needs to live under the Monaco skin.

IIRC there were some issues with that, but I might be mistaken. In any case, I'm sure you'll be able to fix such issues.

Running git grep --text cleared through the social tools extensions only seemed to give one result which was in BlogPage:
[...]
I was actually expecting more results, so I'll run through them again just in case.

The fact that you even got this one surprises me, because I thought I had killed the cleared class a while ago. :) In August 2015 I did a bunch of commits like this one to kill the cleared class for good, but apparently the four instances you mentioned in BlogPage somehow escaped that. Let's fix that.

(For the record: the shared/ dir in SocialProfile was added in SP version 1.7.2, Change-Id: I05f0afc100072060394fb5d5b626ac23de1c0add, committed on 29 July 2015. Before that pretty much each SP subcomponent reimplemented the cleared and clearfix classes in their CSS files.)

Also, it should be noted that in addition to extensions, a bunch of skins implement the cleared CSS class; some ShoutWiki exclusive, as well as the following FOSS ones:

This is code duplication and thus technical debt, since MediaWiki already provides a CSS class, .visualClear.

That's correct.

shared/clearfix.css
/* Monaco is different */
.skin-monaco .clearfix { display: block; }

From my understanding, this needs to live under the Monaco skin.

IIRC there were some issues with that, but I might be mistaken. In any case, I'm sure you'll be able to fix such issues.

Monaco is closed-source, remember? :-) Let's talk about it on IRC.

Running git grep --text cleared through the social tools extensions only seemed to give one result which was in BlogPage:
[...]
I was actually expecting more results, so I'll run through them again just in case.

The fact that you even got this one surprises me, because I thought I had killed the cleared class a while ago. :) In August 2015 I did a bunch of commits like this one to kill the cleared class for good, but apparently the four instances you mentioned in BlogPage somehow escaped that. Let's fix that.

https://gerrit.wikimedia.org/r/#/c/334804/ :D

(For the record: the shared/ dir in SocialProfile was added in SP version 1.7.2, Change-Id: I05f0afc100072060394fb5d5b626ac23de1c0add, committed on 29 July 2015. Before that pretty much each SP subcomponent reimplemented the cleared and clearfix classes in their CSS files.)

Also, it should be noted that in addition to extensions, a bunch of skins implement the cleared CSS class; some ShoutWiki exclusive, as well as the following FOSS ones:

Sure, I'll start pushing out some patches to those other skins!

Ran git grep clearfix, more results this time:
FanBoxes

FanBoxPage.php:         $output .= '<div class="fanbox-page-container clearfix">' .
SpecialViewFanBoxes.php:                        <div class="view-fanboxes-container clearfix">';
UserBoxesHook.php:                      $output .= '<div class="clearfix"><div class="user-fanbox-container">';

LinkFilter

LinkPage.php:           $wgOut->addHTML( '<div id="link-page-container" class="clearfix">' );

SocialProfile seems to really like this class:

SocialProfile.php:$wgResourceModules['ext.socialprofile.clearfix'] = array(
SocialProfile.php:      'styles' => 'clearfix.css',
UserBoard/SpecialSendBoardBlast.php:                    'ext.socialprofile.clear                                                                                                                fix',
UserBoard/SpecialUserBoard.php:                 'ext.socialprofile.clearfix',
UserProfile/SpecialEditProfile.php:                     'ext.socialprofile.clear                                                                                                                fix',
UserProfile/SpecialUpdateProfile.php:                   'ext.socialprofile.clear                                                                                                                fix',
UserProfile/SpecialUpdateProfile.php:           $form .= '<div class="profile-in                                                                                                                fo clearfix">';
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info                                                                                                                 profile-info-other-info clearfix">
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info                                                                                                                 clearfix">
UserProfile/SpecialUpdateProfile.php:           $form .= '<div class="profile-in                                                                                                                fo clearfix">
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info                                                                                                                 profile-info-custom-info clearfix">
UserProfile/SpecialUploadAvatar.php:                    'ext.socialprofile.clear                                                                                                                fix',
UserProfile/UserProfileHooks.php:                               'ext.socialprofi                                                                                                                le.clearfix',
:...skipping...
SocialProfile.php:$wgResourceModules['ext.socialprofile.clearfix'] = array(
SocialProfile.php:      'styles' => 'clearfix.css',
UserBoard/SpecialSendBoardBlast.php:                    'ext.socialprofile.clearfix',
UserBoard/SpecialUserBoard.php:                 'ext.socialprofile.clearfix',
UserProfile/SpecialEditProfile.php:                     'ext.socialprofile.clearfix',
UserProfile/SpecialUpdateProfile.php:                   'ext.socialprofile.clearfix',
UserProfile/SpecialUpdateProfile.php:           $form .= '<div class="profile-info clearfix">';
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info profile-info-other-info clearfix">
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info clearfix">
UserProfile/SpecialUpdateProfile.php:           $form .= '<div class="profile-info clearfix">
UserProfile/SpecialUpdateProfile.php:                   <div class="profile-info profile-info-custom-info clearfix">
UserProfile/SpecialUploadAvatar.php:                    'ext.socialprofile.clearfix',
UserProfile/UserProfileHooks.php:                               'ext.socialprofile.clearfix',
UserProfile/UserProfilePage.php:                $wgOut->addHTML( '<div id="user-page-left" class="clearfix">' );
UserProfile/UserProfilePage.php:                $wgOut->addHTML( '<div id="user-page-right" class="clearfix">' );
UserProfile/UserProfilePage.php:                        <div class="user-fanbox-container clearfix">';
UserRelationship/SpecialRemoveRelationship.php:                 'ext.socialprofile.clearfix',
UserRelationship/SpecialViewRelationshipRequests.php:                   'ext.socialprofile.clearfix',
UserRelationship/SpecialViewRelationships.php:                  'ext.socialprofile.clearfix',
shared/clearfix.css:.clearfix:after {
shared/clearfix.css:.clearfix { display: inline-block; }
shared/clearfix.css:.skin-monaco .clearfix { display: block; }

Change 334854 had a related patch set uploaded (by SamanthaNguyen):
Remove unused module that's being loaded on several special pages

https://gerrit.wikimedia.org/r/334854

Change 334804 merged by jenkins-bot:
v 2.4.6 - Replace custom class with core-provided class

https://gerrit.wikimedia.org/r/334804

Change 334854 merged by jenkins-bot:
Remove unused module that's being loaded on several special pages

https://gerrit.wikimedia.org/r/334854

Change 334855 had a related patch set uploaded (by SamanthaNguyen):
Remove unused module from one final page that was accidentally leftover

https://gerrit.wikimedia.org/r/334855

Change 334855 merged by jenkins-bot:
Remove unused module from one final page that was accidentally leftover

https://gerrit.wikimedia.org/r/334855

Change 336046 had a related patch set uploaded (by SamanthaNguyen):
v 1.1.0 - Replace custom class with core-provided class

https://gerrit.wikimedia.org/r/336046

Change 336046 merged by jenkins-bot:
v 1.1.0 - Replace custom class with core-provided class

https://gerrit.wikimedia.org/r/336046

Change 336051 had a related patch set uploaded (by SamanthaNguyen):
Remove social-tools clearfix technical debt, part 1

https://gerrit.wikimedia.org/r/336051

Change 336051 abandoned by SamanthaNguyen:
Remove social-tools clearfix technical debt, part 1

Reason:
Accidental commit, already done in I952135802fe915933969750336f6b1937ec8b86d

https://gerrit.wikimedia.org/r/336051

Change 885455 had a related patch set uploaded (by Jack Phoenix; author: Jack Phoenix):

[mediawiki/skins/Nimbus@master] Rename "cleared" to the standard name of "visualClear"

https://gerrit.wikimedia.org/r/885455

Change 885455 merged by jenkins-bot:

[mediawiki/skins/Nimbus@master] Rename "cleared" to the standard name of "visualClear"

https://gerrit.wikimedia.org/r/885455