r104388 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r104387‎ | r104388 | r104389 >
Date:05:00, 28 November 2011
Author:varnent
Status:deferred (Comments)
Tags:
Comment:
improvement in use of arrays per feedback to r104384 - ty Johnduhart
Modified paths:
  • /trunk/extensions/AddThis/AddThis.body.php (modified) (history)
  • /trunk/extensions/AddThis/AddThis.php (modified) (history)
  • /trunk/extensions/AddThis/addThis.css (modified) (history)

Diff [purge]

Index: trunk/extensions/AddThis/AddThis.body.php
@@ -19,16 +19,11 @@
2020 # Output AddThis widget
2121 $output .='<!-- AddThis Button BEGIN -->
2222 <div class="addthis_toolbox addthis_default_style" id="addthistoolbar" style="background:'.$wgAddThisBackground.'; border-color:'.$wgAddThisBorder.';">
23 - <a href="http://www.addthis.com/bookmark.php?v=250&amp;pubid=ra-4eb75def4ec6488b" class="addthis_button_compact">&nbsp;' . $share . '</a><span class="addthis_separator">&nbsp;</span>
24 - <a class="addthis_button_'.$wgAddThisHServ[1].'" '.$wgAddThisHServ['1set'].'></a>
25 - <a class="addthis_button_'.$wgAddThisHServ[2].'" '.$wgAddThisHServ['2set'].'></a>
26 - <a class="addthis_button_'.$wgAddThisHServ[3].'" '.$wgAddThisHServ['3set'].'></a>
27 - <a class="addthis_button_'.$wgAddThisHServ[4].'" '.$wgAddThisHServ['4set'].'></a>
28 - <a class="addthis_button_'.$wgAddThisHServ[5].'" '.$wgAddThisHServ['5set'].'></a>
29 - <a class="addthis_button_'.$wgAddThisHServ[6].'" '.$wgAddThisHServ['6set'].'></a>
30 - <a class="addthis_button_'.$wgAddThisHServ[7].'" '.$wgAddThisHServ['7set'].'></a>
31 - <a class="addthis_button_'.$wgAddThisHServ[8].'" '.$wgAddThisHServ['8set'].'></a>
32 - </div>
 23+ <a href="http://www.addthis.com/bookmark.php?v=250&amp;pubid=ra-4eb75def4ec6488b" class="addthis_button_compact">&nbsp;' . $share . '</a><span class="addthis_separator">&nbsp;</span>';
 24+ foreach ( $wgAddThisHServ as $n => $a ) {
 25+ $output .= '<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'" '.$wgAddThisHServ[$n]["attribs"].'></a>';
 26+ }
 27+ $output .='</div>
3328 <script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
3429 <!-- AddThis Button END -->';
3530 return $output;
@@ -47,13 +42,11 @@
4843 if(strtolower($wgAddThisSidebar) == 'true') {
4944 # Output AddThis widget
5045 $bar['addthis'] = '<!-- AddThis Button BEGIN -->
51 - <div class="addthis_toolbox addthis_default_style" id="addthissidebar">
52 - <a class="addthis_button_'.$wgAddThisSBServ[1].'" '.$wgAddThisSBServ['1set'].'></a>
53 - <a class="addthis_button_'.$wgAddThisSBServ[2].'" '.$wgAddThisSBServ['2set'].'></a>
54 - <a class="addthis_button_'.$wgAddThisSBServ[3].'" '.$wgAddThisSBServ['3set'].'></a>
55 - <a class="addthis_button_'.$wgAddThisSBServ[4].'" '.$wgAddThisSBServ['4set'].'></a>
56 - <a class="addthis_button_'.$wgAddThisSBServ[5].'" '.$wgAddThisSBServ['5set'].'></a>
57 - </div>
 46+ <div class="addthis_toolbox addthis_default_style" id="addthissidebar">';
 47+ foreach ( $wgAddThisSBServ as $n => $a ) {
 48+ $bar['addthis'] .= '<a class="addthis_button_'.$wgAddThisSBServ[$n]["service"].'" '.$wgAddThisSBServ[$n]["attribs"].'></a>';
 49+ }
 50+ $bar['addthis'] .= '</div>
5851 <script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
5952 <!-- AddThis Button END -->';
6053 return $bar;
@@ -85,17 +78,12 @@
8679 if ( MWNamespace::isContent( $title->getNamespace() ) ) {
8780 # Output AddThis widget
8881 $wgOut->addHTML('<!-- AddThis Button BEGIN -->
89 - <div class="addthis_toolbox addthis_default_style" id="addthisheader" style="background:'.$wgAddThisBackground.'; border-color:'.$wgAddThisBorder.';">
90 - <a href="http://www.addthis.com/bookmark.php?v=250&amp;pubid=ra-4eb75def4ec6488b" class="addthis_button_compact">&nbsp;' . $share . '</a><span class="addthis_separator">&nbsp;</span>
91 - <a class="addthis_button_'.$wgAddThisHServ[1].'" '.$wgAddThisHServ['1set'].'></a>
92 - <a class="addthis_button_'.$wgAddThisHServ[2].'" '.$wgAddThisHServ['2set'].'></a>
93 - <a class="addthis_button_'.$wgAddThisHServ[3].'" '.$wgAddThisHServ['3set'].'></a>
94 - <a class="addthis_button_'.$wgAddThisHServ[4].'" '.$wgAddThisHServ['4set'].'></a>
95 - <a class="addthis_button_'.$wgAddThisHServ[5].'" '.$wgAddThisHServ['5set'].'></a>
96 - <a class="addthis_button_'.$wgAddThisHServ[6].'" '.$wgAddThisHServ['6set'].'></a>
97 - <a class="addthis_button_'.$wgAddThisHServ[7].'" '.$wgAddThisHServ['7set'].'></a>
98 - <a class="addthis_button_'.$wgAddThisHServ[8].'" '.$wgAddThisHServ['8set'].'></a>
99 - </div>
 82+ <div class="addthis_toolbox addthis_default_style" id="addthistoolbar" style="background:'.$wgAddThisBackground.'; border-color:'.$wgAddThisBorder.';">
 83+ <a href="http://www.addthis.com/bookmark.php?v=250&amp;pubid=ra-4eb75def4ec6488b" class="addthis_button_compact">&nbsp;' . $share . '</a><span class="addthis_separator">&nbsp;</span>');
 84+ foreach ( $wgAddThisHServ as $n => $a ) {
 85+ $wgOut->addHTML('<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'" '.$wgAddThisHServ[$n]["attribs"].'></a>');
 86+ }
 87+ $wgOut->addHTML('</div>
10088 <script type="text/javascript" src="http://s7.addthis.com/js/250/addthis_widget.js#pubid='.$wgAddThispubid.'"></script>
10189 <!-- AddThis Button END -->');
10290 }
Index: trunk/extensions/AddThis/AddThis.php
@@ -43,72 +43,72 @@
4444 * $wgAddThisMain
4545 * - Display AddThis widget on main page
4646 * Default is true
47 - * $wgAddThisSBServ[1]
 47+ * $wgAddThisSBServ[0]['service']
4848 * - Service code for 1st button in sidebar - service codes: http://www.addthis.com/services/list
4949 * Default is compact - AddThis icon used to access full AddThis popup menu
50 - * $wgAddThisSBServ['1set']
 50+ * $wgAddThisSBServ[0]['attribs']
5151 * - Settings for 1st button in sidebar - more info: http://www.addthis.com/help/client-api#attribute-config
52 - * $wgAddThisSBServ[2]
 52+ * $wgAddThisSBServ[1]['service']
5353 * - Service code for 2nd button in sidebar
5454 * Default is facebook
55 - * $wgAddThisSBServ['2set']
 55+ * $wgAddThisSBServ[1]['attribs']
5656 * - Settings for 2nd button in sidebar
57 - * $wgAddThisSBServ[3]
 57+ * $wgAddThisSBServ[2]['service']
5858 * - Service code for 3rd button in sidebar
5959 * Default is twitter
60 - * $wgAddThisSBServ['3set']
 60+ * $wgAddThisSBServ[2]['attribs']
6161 * - Settings for 3rd button in sidebar
62 - * $wgAddThisSBServ[4]
 62+ * $wgAddThisSBServ[3]['service']
6363 * - Service code for 4th button in sidebar
6464 * Default is google_plusone
65 - * $wgAddThisSBServ['4set']
 65+ * $wgAddThisSBServ[3]['attribs']
6666 * - Settings for 4th button in sidebar
6767 * Default is g:plusone:count="false" style="margin-top:1px;"
68 - * $wgAddThisSBServ[5]
 68+ * $wgAddThisSBServ[4]['service']
6969 * - Service code for 5th button in sidebar
7070 * Default is email
71 - * $wgAddThisSBServ['5set']
 71+ * $wgAddThisSBServ[4]['attribs']
7272 * - Settings for 5th button in sidebar
73 - * $wgAddThisHServ[1]
 73+ * $wgAddThisHServ[0]['service']
7474 * - Service code for 1st button in article header after AddThis icon (which cannot be moved in the header)
7575 * Default is facebook
76 - * $wgAddThisHServ['1set']
 76+ * $wgAddThisHServ[0]['attribs']
7777 * - Settings for 1st button in article header
78 - * $wgAddThisHServ[2]
 78+ * $wgAddThisHServ[1]['service']
7979 * - Service code for 2nd button in article header
8080 * Default is twitter
81 - * $wgAddThisHServ['2set']
 81+ * $wgAddThisHServ[1]['attribs']
8282 * - Settings for 2nd button in article header
83 - * $wgAddThisHServ[3]
 83+ * $wgAddThisHServ[2]['service']
8484 * - Service code for 3rd button in article header
8585 * Default is google_plusone
86 - * $wgAddThisHServ['3set']
 86+ * $wgAddThisHServ[2]['attribs']
8787 * - Settings for 3rd button in article header
8888 * Default is g:plusone:count="false" style="margin-top:1px;"
89 - * $wgAddThisHServ[4]
 89+ * $wgAddThisHServ[3]['service']
9090 * - Service code for 4th button in article header
9191 * Default is linkedin
92 - * $wgAddThisHServ['4set']
 92+ * $wgAddThisHServ[3]['attribs']
9393 * - Settings for 4th button in article header
94 - * $wgAddThisHServ[5]
 94+ * $wgAddThisHServ[4]['service']
9595 * - Service code for 5th button in article header
9696 * Default is tumblr
97 - * $wgAddThisHServ['5set']
 97+ * $wgAddThisHServ[4]['attribs']
9898 * - Settings for 5th button in article header
99 - * $wgAddThisHServ[6]
 99+ * $wgAddThisHServ[5]['service']
100100 * - Service code for 6th button in article header
101101 * Default is stumbleupon
102 - * $wgAddThisHServ['6set']
 102+ * $wgAddThisHServ[5]['attribs']
103103 * - Settings for 6th button in article header
104 - * $wgAddThisHServ[7]
 104+ * $wgAddThisHServ[6]['service']
105105 * - Service code for 7th button in article header
106106 * Default is reddit
107 - * $wgAddThisHServ['7set']
 107+ * $wgAddThisHServ[6]['attribs']
108108 * - Settings for 7th button in article header
109 - * $wgAddThisHServ[8]
 109+ * $wgAddThisHServ[7]['service']
110110 * - Service code for 8th button in article header
111111 * Default is email
112 - * $wgAddThisHServ['8set']
 112+ * $wgAddThisHServ[7]['attribs']
113113 * - Settings for 8th button in article header
114114 */
115115
@@ -123,36 +123,51 @@
124124
125125 # Sidebar settings
126126 $wgAddThisSBServ = array(
127 - 1 => 'compact',
128 - '1set' => '',
129 - 2 => 'facebook',
130 - '2set' => '',
131 - 3 => 'twitter',
132 - '3set' => '',
133 - 4 => 'google_plusone',
134 - '4set' => 'g:plusone:count="false" style="margin-top:1px;"',
135 - 5 => 'email',
136 - '5set' => '',
 127+ array(
 128+ 'service' => 'compact',
 129+ ),
 130+ array(
 131+ 'service' => 'facebook',
 132+ ),
 133+ array(
 134+ 'service' => 'twitter',
 135+ ),
 136+ array(
 137+ 'service' => 'google_plusone',
 138+ 'attribs' => 'g:plusone:count="false" style="margin-top:1px;"',
 139+ ),
 140+ array(
 141+ 'service' => 'email',
 142+ ),
137143 );
138144
139145 # Toolbar settings
140146 $wgAddThisHServ = array(
141 - 1 => 'facebook',
142 - '1set' => '',
143 - 2 => 'twitter',
144 - '2set' => '',
145 - 3 => 'google_plusone',
146 - '3set' => 'g:plusone:count="false" style="margin-top:1px;"',
147 - 4 => 'linkedin',
148 - '4set' => '',
149 - 5 => 'tumblr',
150 - '5set' => '',
151 - 6 => 'stumbleupon',
152 - '6set' => '',
153 - 7 => 'reddit',
154 - '7set' => '',
155 - 8 => 'email',
156 - '8set' => '',
 147+ array(
 148+ 'service' => 'facebook',
 149+ ),
 150+ array(
 151+ 'service' => 'twitter',
 152+ ),
 153+ array(
 154+ 'service' => 'google_plusone',
 155+ 'attribs' => 'g:plusone:count="false" style="margin-top:1px;"',
 156+ ),
 157+ array(
 158+ 'service' => 'linkedin',
 159+ ),
 160+ array(
 161+ 'service' => 'tumblr',
 162+ ),
 163+ array(
 164+ 'service' => 'stumbleupon',
 165+ ),
 166+ array(
 167+ 'service' => 'reddit',
 168+ ),
 169+ array(
 170+ 'service' => 'email',
 171+ ),
157172 );
158173
159174
@@ -162,7 +177,7 @@
163178 */
164179 $wgExtensionCredits['other'][] = array(
165180 'name' => 'AddThis',
166 - 'version' => '1.0e',
 181+ 'version' => '1.0f',
167182 'author' => '[http://en.wikipedia.org/wiki/User:Varnent Gregory Varnum]',
168183 'description' => 'Adds [http://www.addthis.com AddThis button] to the sidebar and page header',
169184 'descriptionmsg' => 'addthis-desc',
Index: trunk/extensions/AddThis/addThis.css
@@ -25,7 +25,7 @@
2626 #addthistoolbar {
2727 float:right;
2828 padding:5px;
29 - padding-top:5px;
 29+ padding-top:10px;
3030 margin-left:10px;
3131 margin-bottom:10px;
3232 border-style:solid;

Follow-up revisions

RevisionCommit summaryAuthorDate
r104394fix for possible PHP notices caused by r104388varnent05:37, 28 November 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r104384switched to arrays in response to r104381#c26594 - ty Johnduhartvarnent00:51, 28 November 2011

Comments

#Comment by Johnduhart (talk | contribs)   05:02, 28 November 2011
+		foreach ( $wgAddThisHServ as $n => $a ) {
+			$output .= '<a class="addthis_button_'.$wgAddThisHServ[$n]["service"].'" '.$wgAddThisHServ[$n]["attribs"].'></a>';
+		}

This will cause PHP notices since attribs won't always be set, check with isset().

#Comment by Nikerabbit (talk | contribs)   09:04, 29 November 2011

And instead of $wgAddThisHServ[$n] inside the loop you can use $a['service'] and give $n and $a better names.

Status & tagging log