r82948 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r82947‎ | r82948 | r82949 >
Date:18:43, 28 February 2011
Author:happy-melon
Status:ok (Comments)
Tags:
Comment:
Rm nonstandard and redundant styles for Special:SpecialPages; really no need for them, plus using an out-of-order header level is bad for accessibility.
Modified paths:
  • /trunk/phase3/includes/specials/SpecialSpecialpages.php (modified) (history)
  • /trunk/phase3/skins/common/shared.css (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/common/shared.css
——@@ -453,16 +453,6 @@
454454 text-align: right;
455455 }
456456
457 -/* Special:SpecialPages styling */
458 -h4.mw-specialpagesgroup {
459 - background-color: #dcdcdc;
460 - padding: 2px;
461 - margin: .3em 0em 0em 0em;
462 -}
463 -.mw-specialpagerestricted {
464 - font-weight: bold;
465 -}
466 -
467457 #shared-image-dup, #shared-image-conflict {
468458 font-style: italic;
469459 }
Index: trunk/phase3/includes/specials/SpecialSpecialpages.php
——@@ -97,7 +97,7 @@
9898 $total = count( $sortedPages );
9999 $count = 0;
100100
101 - $wgOut->wrapWikiMsg( "<h4 class=\"mw-specialpagesgroup\" id=\"mw-specialpagesgroup-$group\">$1</h4>\n", "specialpages-group-$group" );
 101+ $wgOut->wrapWikiMsg( "<h2 class=\"mw-specialpagesgroup\" id=\"mw-specialpagesgroup-$group\">$1</h2>\n", "specialpages-group-$group" );
102102 $wgOut->addHTML(
103103 Html::openElement( 'table', array( 'style' => 'width:100%;', 'class' => 'mw-specialpages-table' ) ) ."\n" .
104104 Html::openElement( 'tr' ) . "\n" .

Follow-up revisions

RevisionCommit summaryAuthorDate
r89767Attempt at fixing the issue raised in r82948.nikerabbit10:43, 9 June 2011

Comments

#Comment by Raymond (talk | contribs)   19:31, 28 February 2011

Is bumping $wgStyleVersion still needed since we have the RL?

#Comment by Catrope (talk | contribs)   19:33, 28 February 2011

No.

#Comment by Krinkle (talk | contribs)   19:37, 28 February 2011

It's still referenced in a few places though, that should be figured out at some point:

"wgStyleVersion" in /trunk/phase3

#Comment by Nikerabbit (talk | contribs)   22:10, 28 February 2011

Special:SpecialPages looks more confusing now. The lists are not tied to headings above, and at the very bottom the legend seems to be part of a list at first sight.

#Comment by Happy-melon (talk | contribs)   23:29, 28 February 2011

It might be worth an <hr/> above the legend, I guess; but how is replacing a header style which is unique to this page with one which is seen and acknowledged everywhere else on the wiki result in a sense of "not tied-in-ness"??

#Comment by Nikerabbit (talk | contribs)   07:42, 1 March 2011

There is already a hr, but it is not enough. There needs to be more empty space. Did you look at the page? There is as much as space above and below the header, and the line under the heading further distances it from the list below. Right now the page is just list of alternating big words and lists of pages, instead of lists of pages with a header.

This is an usability issue. In fact I found a blog entry which describes almost identical case.

#Comment by Happy-melon (talk | contribs)   09:35, 1 March 2011

Of course I looked at the page, albeit not when writing that comment. I really fail to see how these points apply to this page any more than to every page on the wiki; indeed the whole point of this commit is to make the styling the same. If you think that the standard h2 headings don't link properly with the content underneath them, then that needs to be changed sitewide, not for this special page.

#Comment by Reach Out to the Truth (talk | contribs)   15:49, 1 March 2011

The table adds additional space between the heading and the first item, so there is a greater disconnect between the heading and the content than usual. I count thirty pixels from the baseline of the heading text to the top of the first list item. Although the actual distance will vary for different people, I do feel that there is too great a disconnect between the heading and content in this situation.

#Comment by Krinkle (talk | contribs)   22:30, 28 February 2011

Removal of- 

.mw-specialpagerestricted {
	font-weight: bold;
}

looked wrong at first, but the html structure is like 

<li class="mw-specialpages-page mw-specialpagerestricted">
 <strong>
  <a href="/wiki/Special:Undelete" title="Special:Undelete">View deleted pages</a>
 </strong>
</li>

So the css declaration is redundant. Just stating here for easier review.

#Comment by Siebrand (talk | contribs)   23:36, 21 June 2011

Removed TODO after r89767.

Status & tagging log