r83863 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r83862‎ | r83863 | r83864 >
Date:23:12, 13 March 2011
Author:saper
Status:ok (Comments)
Tags:
Comment:
Bug 2557: Sidebar items missing from classic, cologne blue, simple skins

* Simple skin support multiple sidebar items
* Standard (a.k.a. Classic) likes to add "my watchlist" and "my contributions"
links at the bottom of the first sidebar box. It also has no titles for boxes.
* Cologne blue supports multiple sidebar boxes with titles.
Modified paths:
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)
  • /trunk/phase3/skins/Standard.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/CologneBlue.php
@@ -213,13 +213,25 @@
214214 unset( $bar['SEARCH'] );
215215 unset( $bar['LANGUAGES'] );
216216 unset( $bar['TOOLBOX'] );
217 - $browseLinks = reset( $bar );
218217
219 - foreach ( $browseLinks as $link ) {
220 - if ( $link['text'] != '-' ) {
221 - $s .= "<a href=\"{$link['href']}\">" .
222 - htmlspecialchars( $link['text'] ) . '</a>' . $sep;
 218+ $barnumber = 1;
 219+ foreach ( $bar as $heading => $browseLinks ) {
 220+ $heading_text = wfMsg ( $heading );
 221+ if ( $barnumber > 1 ) {
 222+ if ( wfEmptyMsg( $heading, $heading_text ) ) {
 223+ $h = $heading;
 224+ } else {
 225+ $h = $heading_text;
 226+ }
 227+ $s .= "\n<h6>" . htmlspecialchars( $h ) . "</h6>";
223228 }
 229+ foreach ( $browseLinks as $link ) {
 230+ if ( $link['text'] != '-' ) {
 231+ $s .= "<a href=\"{$link['href']}\">" .
 232+ htmlspecialchars( $link['text'] ) . '</a>' . $sep;
 233+ }
 234+ }
 235+ $barnumber = $barnumber + 1;
224236 }
225237
226238 if ( $wgOut->isArticle() ) {
Index: trunk/phase3/skins/Standard.php
@@ -118,25 +118,33 @@
119119 unset( $bar['SEARCH'] );
120120 unset( $bar['LANGUAGES'] );
121121 unset( $bar['TOOLBOX'] );
122 - $browseLinks = reset( $bar );
123122
124 - foreach ( $browseLinks as $link ) {
125 - if ( $link['text'] != '-' ) {
126 - $s .= "<a href=\"{$link['href']}\">" .
127 - htmlspecialchars( $link['text'] ) . '</a>' . $sep;
 123+ $barnumber = 1;
 124+ foreach ( $bar as $heading => $browseLinks ) {
 125+ if ( $barnumber > 1 ) {
 126+ $s .= "\n<hr class='sep' />";
 127+ }
 128+ foreach ( $browseLinks as $link ) {
 129+ if ( $link['text'] != '-' ) {
 130+ $s .= "<a href=\"{$link['href']}\">" .
 131+ htmlspecialchars( $link['text'] ) . '</a>' . $sep;
 132+ }
128133 }
 134+ if ( $barnumber == 1 ) {
 135+ // only show watchlist link if logged in
 136+ if( $wgUser->isLoggedIn() ) {
 137+ $s.= $this->getSkin()->specialLink( 'Watchlist' ) ;
 138+ $s .= $sep . $this->getSkin()->linkKnown(
 139+ SpecialPage::getTitleFor( 'Contributions' ),
 140+ wfMsg( 'mycontris' ),
 141+ array(),
 142+ array( 'target' => $wgUser->getName() )
 143+ );
 144+ }
 145+ }
 146+ $barnumber = $barnumber + 1;
129147 }
130148
131 - if( $wgUser->isLoggedIn() ) {
132 - $s.= $this->getSkin()->specialLink( 'Watchlist' ) ;
133 - $s .= $sep . $this->getSkin()->linkKnown(
134 - SpecialPage::getTitleFor( 'Contributions' ),
135 - wfMsg( 'mycontris' ),
136 - array(),
137 - array( 'target' => $wgUser->getName() )
138 - );
139 - }
140 - // only show watchlist link if logged in
141149 $s .= "\n<hr class='sep' />";
142150 $articleExists = $this->getSkin()->getTitle()->getArticleId();
143151 if ( $wgOut->isArticle() || $action == 'edit' || $action == 'history' || $wpPreview ) {

Comments

#Comment by Aaron Schulz (talk | contribs)   22:28, 30 June 2011

Are there Simple skin changes that where supposed to be committed here?

#Comment by Aaron Schulz (talk | contribs)   22:29, 30 June 2011

wfEmptyMsg( $heading, $heading_text )

You don't need the second param anymore.

#Comment by Aaron Schulz (talk | contribs)   22:51, 30 June 2011

I don't like the $barnumber == 1 check in that it now assumes $bar always is not empty (otherwise the watchlist link won't get added). But that's unlikely and that skin isn't really supported much anyway, so it's tolerable.

Status & tagging log