r81914 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r81913‎ | r81914 | r81915 >
Date:19:46, 10 February 2011
Author:platonides
Status:ok (Comments)
Tags:
Comment:
Kill calls to tooltip() and tooltipAndAccesskeyAttribs() from MonoBook and Modern
Modified paths:
  • /trunk/phase3/skins/Modern.php (modified) (history)
  • /trunk/phase3/skins/MonoBook.php (modified) (history)

Diff [purge]

Index: trunk/phase3/skins/MonoBook.php
@@ -96,28 +96,7 @@
9797 </div>
9898 </div></div>
9999 <div id="column-one"<?php $this->html('userlangattributes') ?>>
100 - <div id="p-cactions" class="portlet">
101 - <h5><?php $this->msg('views') ?></h5>
102 - <div class="pBody">
103 - <ul><?php
104 - foreach($this->data['content_actions'] as $key => $tab) {
105 - echo '
106 - <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
107 - if( $tab['class'] ) {
108 - echo ' class="'.htmlspecialchars($tab['class']).'"';
109 - }
110 - echo '><a href="'.htmlspecialchars($tab['href']).'"';
111 - if( $tab["tooltiponly"] ) {
112 - echo $skin->tooltip( "ca-$key" );
113 - } else {
114 - echo $skin->tooltipAndAccesskey( "ca-$key" );
115 - }
116 - echo '>'.htmlspecialchars($tab['text']).'</a></li>';
117 - } ?>
118 -
119 - </ul>
120 - </div>
121 - </div>
 100+<?php $this->cactions( $skin ); ?>
122101 <div class="portlet" id="p-personal">
123102 <h5><?php $this->msg('personaltools') ?></h5>
124103 <div class="pBody">
@@ -213,6 +192,42 @@
214193 <?php
215194 }
216195
 196+ /**
 197+ * Prints the cactions bar.
 198+ * Shared between MonoBook and Modern
 199+ */
 200+ function cactions( Skin $skin ) {
 201+?>
 202+ <div id="p-cactions" class="portlet">
 203+ <h5><?php $this->msg('views') ?></h5>
 204+ <div class="pBody">
 205+ <ul><?php
 206+ foreach($this->data['content_actions'] as $key => $tab) {
 207+ echo '
 208+ <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
 209+ if( $tab['class'] ) {
 210+ echo ' class="'.htmlspecialchars($tab['class']).'"';
 211+ }
 212+ echo '>';
 213+ $linkAttribs = array( 'href' => $tab['href'] );
 214+
 215+ if( isset( $tab["tooltiponly"] ) && $tab["tooltiponly"] ) {
 216+ $title = $skin->titleAttrib( "ca-$key" );
 217+ if ( $title !== false ) {
 218+ $linkAttribs['title'] = $title;
 219+ }
 220+ } else {
 221+ $linkAttribs += $skin->tooltipAndAccesskeyAttribs( "ca-$key" );
 222+ }
 223+ echo Html::element( 'a', $linkAttribs, $tab['text'] );
 224+ echo '</li>';
 225+ } ?>
 226+
 227+ </ul>
 228+ </div>
 229+ </div>
 230+<?php
 231+ }
217232 /*************************************************************************************************/
218233 function toolbox() {
219234 ?>
Index: trunk/phase3/skins/Modern.php
@@ -58,27 +58,7 @@
5959 <div id="mw_main">
6060 <div id="mw_contentwrapper">
6161 <!-- navigation portlet -->
62 - <div id="p-cactions" class="portlet">
63 - <h5><?php $this->msg('views') ?></h5>
64 - <div class="pBody">
65 - <ul>
66 - <?php foreach($this->data['content_actions'] as $key => $tab) {
67 - echo '
68 - <li id="' . Sanitizer::escapeId( "ca-$key" ) . '"';
69 - if( $tab['class'] ) {
70 - echo ' class="'.htmlspecialchars($tab['class']).'"';
71 - }
72 - echo'><a href="'.htmlspecialchars($tab['href']).'"';
73 - if( isset($tab["tooltiponly"]) && $tab["tooltiponly"] ) {
74 - echo $skin->tooltip( "ca-$key" );
75 - } else {
76 - echo $skin->tooltipAndAccesskey( "ca-$key" );
77 - }
78 - echo '>'.htmlspecialchars($tab['text']).'</a></li>';
79 - } ?>
80 - </ul>
81 - </div>
82 - </div>
 62+<?php $this->cactions( $skin ); ?>
8363
8464 <!-- content -->
8565 <div id="mw_content">

Follow-up revisions

RevisionCommit summaryAuthorDate
r81915Follow up r81914. Don't build the <li> manually.platonides20:36, 10 February 2011

Comments

#Comment by Dantman (talk | contribs)   20:39, 10 February 2011
  • sigh* This is not really what I'd consider skin improvement. content_actions has been on my list to improve in a more flexible way, iirc I needed to make some tweaks to the link/list helpers.
#Comment by Platonides (talk | contribs)   20:52, 10 February 2011

I was just attacking the deprecated functions. The cactions should have been shareable at a lower level, not just MonoBook and Modern.

#Comment by Dantman (talk | contribs)   21:00, 10 February 2011

The plan was to use make use of makeListItem just like personaltools, toolbox, language_urls, and the rest of the sidebar... just needed some small tweaks and a helper. The delay in comparison to the others was due to content_actions vs. cotnent_navigation (likely needing some 'flag' style helper or something) and possibly a little due to the tooltip+acceskey vs. just tooltip bit.

Status & tagging log