r108039 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r108038‎ | r108039 | r108040 >
Date:15:27, 4 January 2012
Author:rsterbin
Status:resolved (Comments)
Tags:aft 
Comment:
Added link option D:
- modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js:
- Added link option D
- Added tracking ID "bottomright-link"
- modules/ext.articleFeedbackv5/ext.articleFeedbackv5.css:
- Added styles for link option D (mostly copied from option C, without
the rotation)
- ArticleFeedbackv5.php:
- Updated options for $wgArticleFeedbackv5LinkBuckets
- ArticleFeedbackv5.i18n.php:
- Added "articlefeedbackv5-bottomrighttab-linktext" for link option D
- ArticleFeedbackv5.hooks.php:
- Added "articlefeedbackv5-bottomrighttab-linktext" to messages for
ext.articleFeedbackv5
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
@@ -188,6 +188,7 @@
189189 'articlefeedbackv5-sitesub-linktext' => 'Improve this page',
190190 'articlefeedbackv5-titlebar-linktext' => 'Help improve this page >>',
191191 'articlefeedbackv5-fixedtab-linktext' => 'Improve this page',
 192+ 'articlefeedbackv5-bottomrighttab-linktext' => 'Improve this page',
192193 'articlefeedbackv5-section-linktext' => 'feedback',
193194 'articlefeedbackv5-toolbox-linktext' => 'Improve this page',
194195
@@ -377,6 +378,7 @@
378379 'articlefeedbackv5-sitesub-linktext' => 'When a link to pop up the feedback tool appears just below the title bar to the far left, this will be the link text.',
379380 'articlefeedbackv5-titlebar-linktext' => 'When a link to pop up the feedback tool appears just below the title bar to the far right, this will be the link text.',
380381 'articlefeedbackv5-fixedtab-linktext' => 'When a link to pop up the feedback tool appears as a fixed-positioned tab, this will be the link text',
 382+ 'articlefeedbackv5-bottomrighttab-linktext' => 'When a link to pop up the feedback tool appears as a fixed-positioned tab at the bottom right, this will be the link text',
381383 'articlefeedbackv5-section-linktext' => 'When a link to pop up the feedback tool appears on the article section headers, next to [edit], this will be the text inside the brackets immediately following (e.g. "[edit] [feedback]").
382384 {{Identical|Feedback}}',
383385 'articlefeedbackv5-toolbox-linktext' => 'When a link to pop up the feedback tool appears at the bottom of the toolbox area in the sidebar, this will be the link text.',
Index: trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.css
@@ -60,3 +60,31 @@
6161 text-decoration: none;
6262 }
6363
 64+.articleFeedbackv5-bottomrighttab {
 65+ position: fixed;
 66+ margin: 0;
 67+ right: 0;
 68+ bottom: 0;
 69+ height: 27px;
 70+ width: 141px;
 71+}
 72+.articleFeedbackv5-bottomrighttabbox {
 73+ position: relative;
 74+}
 75+.articleFeedbackv5-bottomrighttablink {
 76+ border: none;
 77+ color: #fff;
 78+ background-color: #5373a6;
 79+ min-width: 105px;
 80+ right: 0;
 81+ display: block;
 82+ margin: 0;
 83+ padding: 5px 10px;
 84+ text-align: center;
 85+ position: absolute;
 86+ z-index: 100000;
 87+}
 88+.articleFeedbackv5-bottomrighttablink:hover {
 89+ text-decoration: none;
 90+}
 91+
Index: trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.js
@@ -23,6 +23,7 @@
2424 'A': { trackId: 'sitesub-link' },
2525 'B': { trackId: 'titlebar-link' },
2626 'C': { trackId: 'vertical-link' },
 27+ 'D': { trackId: 'bottomright-link' },
2728 'H': { trackId: 'section-link' },
2829 'tbx': { trackId: 'toolbox-link' }
2930 };
@@ -132,7 +133,26 @@
133134 }
134135
135136 // D: Button fixed to bottom right
136 -// NOT IMPLEMENTED
 137+if ( 'D' == linkBucket ) {
 138+ var $bottomRightTab = $( '\
 139+ <div id="articleFeedbackv5-bottomrighttab">\
 140+ <div id="articleFeedbackv5-bottomrighttabbox">\
 141+ <a href="#mw-articleFeedbackv5" id="articleFeedbackv5-bottomrighttablink"></a>\
 142+ </div>\
 143+ </div>' );
 144+ $bottomRightTab.find( '#articleFeedbackv5-bottomrighttablink' )
 145+ .data( 'linkId', 'D' )
 146+ .html( mw.msg( 'articlefeedbackv5-bottomrighttab-linktext' ) )
 147+ .click( function( e ) {
 148+ e.preventDefault();
 149+ clickFeedbackLink( $( e.target ) );
 150+ } );
 151+ $bottomRightTab.insertBefore( $aftDiv );
 152+ $bottomRightTab.addClass( 'articleFeedbackv5-bottomrighttab' );
 153+ $bottomRightTab.find( '#articleFeedbackv5-bottomrighttabbox' ).addClass( 'articleFeedbackv5-bottomrighttabbox' );
 154+ $bottomRightTab.find( '#articleFeedbackv5-bottomrighttablink' ).addClass( 'articleFeedbackv5-bottomrighttablink' );
 155+ $aftDiv.articleFeedbackv5( 'addToRemovalQueue', $bottomRightTab );
 156+}
137157
138158 // E: Button fixed to bottom center
139159 // NOT IMPLEMENTED
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.php
@@ -128,11 +128,11 @@
129129 // G: Button below logo
130130 // H: Link on each section bar
131131 'buckets' => array(
132 - '-' => 34,
133 - 'A' => 33,
134 - 'B' => 0,
 132+ '-' => 25,
 133+ 'A' => 25,
 134+ 'B' => 25,
135135 'C' => 0,
136 - 'D' => 33,
 136+ 'D' => 25,
137137 'E' => 0,
138138 'F' => 0,
139139 'G' => 0,
Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.hooks.php
@@ -27,6 +27,7 @@
2828 'articlefeedbackv5-sitesub-linktext',
2929 'articlefeedbackv5-titlebar-linktext',
3030 'articlefeedbackv5-fixedtab-linktext',
 31+ 'articlefeedbackv5-bottomrighttab-linktext',
3132 'articlefeedbackv5-section-linktext',
3233 'articlefeedbackv5-toolbox-linktext',
3334 'articlefeedbackv5-bucket5-toolbox-linktext',

Follow-up revisions

RevisionCommit summaryAuthorDate
r108512Fix for r108039; link text should be escapedrsterbin15:55, 10 January 2012

Comments

#Comment by Catrope (talk | contribs)   11:42, 10 January 2012
+		.html( mw.msg( 'articlefeedbackv5-bottomrighttab-linktext' ) )

Should use .text() .

+	$bottomRightTab.addClass( 'articleFeedbackv5-bottomrighttab' );
+	$bottomRightTab.find( '#articleFeedbackv5-bottomrighttabbox' ).addClass( 'articleFeedbackv5-bottomrighttabbox' );
+	$bottomRightTab.find( '#articleFeedbackv5-bottomrighttablink' ).addClass( 'articleFeedbackv5-bottomrighttablink' );

Why are you adding these classes using jQuery, can't you add them in the HTML you're using to construct $bottomRightTab instead?

Marking fixme for the .html()/.text() thing, OK otherwise.

#Comment by Rsterbin (talk | contribs)   15:57, 10 January 2012

I've fixed the .html()/.text() issue. Adding classes with .addClass() rather than in the html was part of the group of fixes Yoni did for IE7; it's not necessary now that IE7 is off, but eventually we will want to turn it back on, and it'll be nice not to have to re-fix the tabs.

Status & tagging log