r105744 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105743‎ | r105744 | r105745 >
Date:02:07, 10 December 2011
Author:rsterbin
Status:ok
Tags:
Comment:
Updates for the jquery module, mostly related to CTAs:
- Made the confirmation message act as the title for CTAs
- Tooltips now work when you're in a CTA
- Structural changes to make it easier for me to load the CTA or the form
as needed (controlled manually on the console for now; later versions will
have a url override):
- New properties toDisplay and hasContainers
- New edit CTA template titleConfirm
- Updated edit CTA's getTitle() to build and localize the confirmation
message before sending it back as an html string
- Renamed loadForm() to showForm()
- Moved building of containers into its own method loadContainers()
- Updated showCTA() to set a custom class on its ui block, change the title
in both places, and reset the dialog dimensions
- Replaced calls to loadForm() with calls to load()
- Moved clearing of the containers out of clear() and into a new method
clearContainers() so that load() can use it too
- Updated markShowstopperError() to indicate that the tool is now showing
an error
- Moved setting the dialog dimensions out of openAsModal() into its own
method setDialogDimensions() so that showCTA() can use it too
- Removed currentDefaultText from the $ scope and put it in the bucket 2
properties where it belongs
- Dropped unused code for positioning the dialog
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css
——@@ -849,20 +849,21 @@
850850 float: left;
851851 width: 340px;
852852 }
853 -.articleFeedbackv5-confirmation-panel .articleFeedbackv5-panel-leftContent .articleFeedbackv5-confirmation-text {
 853+.articleFeedbackv5-confirmation-text {
854854 display: inline-block;
855855 background: #f7f5f5;
856856 border: 1px solid #cccaca;
857857 padding: 5px 10px;
858858 line-height: 25px;
859 -
 859+ font-size: 0.75em;
 860+ font-weight: normal;
860861 }
861 -.articleFeedbackv5-confirmation-panel .articleFeedbackv5-panel-leftContent .articleFeedbackv5-confirmation-text .articleFeedbackv5-confirmation-thanks {
 862+.articleFeedbackv5-confirmation-text .articleFeedbackv5-confirmation-thanks {
862863 font-weight: bold;
863864 font-style: italic;
864865 display: inline-block;
865 - padding: 0 0 0 24px;
866 - margin: 0 9px 0 0;
 866+ padding: 0 0 0 30px;
 867+ margin: 0 10px 0 0;
867868 height: 25px;
868869 /* @embed */
869870 background: url(images/bg-confirmation-thanks.png) 0 center no-repeat;
——@@ -945,7 +946,7 @@
946947 margin-bottom: 5px;
947948 }
948949
949 -/*** CTA has a button to remove itself from the page ***/
 950+/*** CTA overrides ***/
950951
951952 .articleFeedbackv5-clear-trigger {
952953 display: block;
——@@ -965,3 +966,13 @@
966967 cursor: pointer;
967968 text-decoration: none;
968969 }
 970+
 971+.articleFeedbackv5-cta-1 .articleFeedbackv5-tooltip {
 972+ top: -405px;
 973+ right: -68px;
 974+}
 975+
 976+.articleFeedbackv5-panel .articleFeedbackv5-cta-1 h2.articleFeedbackv5-title {
 977+ font-weight: normal;
 978+}
 979+
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js
——@@ -61,6 +61,11 @@
6262 $.articleFeedbackv5.debug = mw.config.get( 'wgArticleFeedbackv5Debug' ) ? true : false;
6363
6464 /**
 65+ * Have the containers been added?
 66+ */
 67+ $.articleFeedbackv5.hasContainers = false;
 68+
 69+ /**
6570 * Has the form been loaded yet?
6671 */
6772 $.articleFeedbackv5.isLoaded = false;
——@@ -132,20 +137,19 @@
133138 $.articleFeedbackv5.revisionId = mw.config.get( 'wgCurRevisionId' );
134139
135140 /**
136 - * Whether we're showing a form, a CTA, or nothing
 141+ * What we're meant to be showing: a form, a CTA, a showstopper error, or nothing
137142 */
 143+ $.articleFeedbackv5.toDisplay = 'form';
 144+
 145+ /**
 146+ * What we're actually showing
 147+ */
138148 $.articleFeedbackv5.nowShowing = 'none';
139149
140150 /**
141151 * The feedback ID (collected on submit, for use in tracking edits)
142152 */
143153 $.articleFeedbackv5.feedbackId = 0;
144 -
145 - /**
146 - * Currently displayed placeholder text for option 2. This is a workaround for Chrome/FF
147 - * behavior overlays.
148 - */
149 - $.currentDefaultText = "";
150154
151155 // }}}
152156 // {{{ Templates
——@@ -461,6 +465,12 @@
462466 */
463467 commentDefault: {},
464468
 469+ /**
 470+ * Currently displayed placeholder text. This is a workaround for Chrome/FF
 471+ * automatic focus in overlays.
 472+ */
 473+ currentDefaultText: '',
 474+
465475 // }}}
466476 // {{{ templates
467477
——@@ -611,7 +621,7 @@
612622 // Clear out the question on focus
613623 $block.find( '.articleFeedbackv5-comment textarea' )
614624 .focus( function () {
615 - if ( $( this ).val() == $.currentDefaultText ) {
 625+ if ( $( this ).val() == $.articleFeedbackv5.currentBucket().currentDefaultText ) {
616626 $( this ).val( '' );
617627 $(this).addClass( 'active' );
618628 }
——@@ -678,14 +688,14 @@
679689 } else {
680690 for ( var t in $.articleFeedbackv5.currentBucket().commentDefault ) {
681691 if ( $c.val() == $.articleFeedbackv5.currentBucket().commentDefault[t] ) {
682 - empty = true;
 692+ empty = true;
683693 }
684694 }
685695 }
686696 if ( empty ) {
687697 $c.val( $.articleFeedbackv5.currentBucket().commentDefault[key] );
688698 // Store default text, workaround for overlay bug in Chrome/FF
689 - $.currentDefaultText = $.articleFeedbackv5.currentBucket().commentDefault[key];
 699+ $.articleFeedbackv5.currentBucket().currentDefaultText = $.articleFeedbackv5.currentBucket().commentDefault[key];
690700 }
691701 } else {
692702 // Clear checked
——@@ -1795,10 +1805,6 @@
17961806 <div class="clear"></div>\
17971807 <div class="articleFeedbackv5-confirmation-panel">\
17981808 <div class="articleFeedbackv5-panel-leftContent">\
1799 - <div class="articleFeedbackv5-confirmation-text">\
1800 - <span class="articleFeedbackv5-confirmation-thanks"><html:msg key="cta1-thanks" /></span>\
1801 - <span class="articleFeedbackv5-confirmation-follow-up"><html:msg key="cta1-confirmation-followup" /></span>\
1802 - </div>\
18031809 <h3 class="articleFeedbackv5-confirmation-title"><html:msg key="cta1-confirmation-title" /></h3>\
18041810 <p class="articleFeedbackv5-confirmation-wikipediaWorks"><html:msg key="cta1-confirmation-call" /></p>\
18051811 <p class="articleFeedbackv5-confirmation-learnHow"><a target="_blank" href="#"><html:msg key="cta1-learn-how" /> &raquo;</a></p>\
——@@ -1806,6 +1812,16 @@
18071813 <a href="&amp;action=edit" class="articleFeedbackv5-edit-cta-link"><span class="ui-button-text"><html:msg key="cta1-edit-linktext" /></span></a>\
18081814 <div class="clear"></div>\
18091815 </div>\
 1816+ ',
 1817+
 1818+ /**
 1819+ * The title/confirmation
 1820+ */
 1821+ titleConfirm: '\
 1822+ <div class="articleFeedbackv5-confirmation-text">\
 1823+ <span class="articleFeedbackv5-confirmation-thanks"><html:msg key="cta1-thanks" /></span>\
 1824+ <span class="articleFeedbackv5-confirmation-follow-up"><html:msg key="cta1-confirmation-followup" /></span>\
 1825+ </div>\
18101826 '
18111827
18121828 },
——@@ -1819,7 +1835,10 @@
18201836 * @return string the title
18211837 */
18221838 getTitle: function () {
1823 -// return 'TODO: EDIT CTA';
 1839+
 1840+ var $title = $( '<div></div>' ).html( $.articleFeedbackv5.currentCTA().templates.titleConfirm );
 1841+ $title.localize( { 'prefix': 'articlefeedbackv5-' } );
 1842+ return $title.html();
18241843 },
18251844
18261845 // }}}
——@@ -1834,7 +1853,7 @@
18351854
18361855 // Start up the block to return
18371856 var $block = $( $.articleFeedbackv5.currentCTA().templates.block );
1838 -
 1857+
18391858 // Fill in the tutorial link
18401859 $block.find( '.articleFeedbackv5-confirmation-learnHow a' )
18411860 .attr( 'href', mw.msg( 'articlefeedbackv5-cta1-learn-how-url' ) );
——@@ -1895,7 +1914,7 @@
18961915 // When the tool is visible, load the form
18971916 $.articleFeedbackv5.$holder.appear( function () {
18981917 if ( !$.articleFeedbackv5.isLoaded ) {
1899 - $.articleFeedbackv5.loadForm();
 1918+ $.articleFeedbackv5.load();
19001919 }
19011920 } );
19021921 // Keep track of links that must be removed after a successful submission
——@@ -2042,7 +2061,7 @@
20432062 if($.articleFeedbackv5.submissionEnabled == state ) {
20442063 return;
20452064 }
2046 -
 2065+
20472066 if ( state ) {
20482067 $.articleFeedbackv5.find( '.articleFeedbackv5-submit' ).button( 'enable' );
20492068 } else {
——@@ -2079,32 +2098,56 @@
20802099 // }}}
20812100 // {{{ Process methods
20822101
2083 - // {{{ loadForm
 2102+ // {{{ load
20842103
20852104 /**
2086 - * Build the form and load it into the document
 2105+ * Loads the tool onto the page
 2106+ *
 2107+ * @param display string "form" or "cta"
20872108 */
2088 - $.articleFeedbackv5.loadForm = function () {
 2109+ $.articleFeedbackv5.load = function ( display ) {
20892110
2090 - // Build the form
2091 - var bucket = $.articleFeedbackv5.currentBucket();
2092 - if ( !( 'buildForm' in bucket ) ) {
2093 - $.articleFeedbackv5.isLoaded = true;
2094 - return;
 2111+ if ( display ) {
 2112+ $.articleFeedbackv5.toDisplay = ( display == 'cta' ? 'cta' : 'form' );
20952113 }
2096 - var $block = bucket.buildForm();
2097 - if ( 'bindEvents' in bucket ) {
2098 - bucket.bindEvents( $block );
 2114+
 2115+ $.articleFeedbackv5.clearContainers();
 2116+ $.articleFeedbackv5.nowShowing = 'none';
 2117+
 2118+ if ( 'form' == $.articleFeedbackv5.toDisplay ) {
 2119+ var bucket = $.articleFeedbackv5.currentBucket();
 2120+ if ( !( 'buildForm' in bucket ) ) {
 2121+ $.articleFeedbackv5.isLoaded = true;
 2122+ return;
 2123+ }
 2124+ $.articleFeedbackv5.loadContainers();
 2125+ $.articleFeedbackv5.showForm();
20992126 }
21002127
2101 - // Wrap it in a panel
 2128+ else if ( 'cta' == $.articleFeedbackv5.toDisplay ) {
 2129+ var cta = $.articleFeedbackv5.currentCTA();
 2130+ if ( !( 'build' in cta ) ) {
 2131+ $.articleFeedbackv5.isLoaded = true;
 2132+ return;
 2133+ }
 2134+ $.articleFeedbackv5.loadContainers();
 2135+ $.articleFeedbackv5.showCTA();
 2136+ }
 2137+
 2138+ };
 2139+
 2140+ // }}}
 2141+ // {{{ loadContainers
 2142+
 2143+ /**
 2144+ * Builds containers and loads them onto the page
 2145+ */
 2146+ $.articleFeedbackv5.loadContainers = function () {
 2147+
 2148+ // Set up the panel
21022149 var $wrapper = $( $.articleFeedbackv5.templates.panelOuter );
2103 - $wrapper.find( '.articleFeedbackv5-ui' )
2104 - .addClass( 'articleFeedbackv5-option-' + $.articleFeedbackv5.bucketId );
2105 - $wrapper.find( '.articleFeedbackv5-ui-inner' )
2106 - .append( $block );
21072150
2108 - // Set up the help tooltip
 2151+ // Add the help tooltip
21092152 $wrapper.find( '.articleFeedbackv5-tooltip-link' )
21102153 .attr( 'href', mw.msg( 'articlefeedbackv5-help-tooltip-linkurl' ) )
21112154 .click( function ( e ) {
——@@ -2116,11 +2159,6 @@
21172160 } );
21182161 $wrapper.find( '.articleFeedbackv5-tooltip' ).hide();
21192162
2120 - // Set the title
2121 - if ( 'getTitle' in bucket ) {
2122 - $wrapper.find( '.articleFeedbackv5-title' ).html( bucket.getTitle() );
2123 - }
2124 -
21252163 // Set up the tooltip trigger for the panel version
21262164 $wrapper.find( '.articleFeedbackv5-title-wrap' ).append( $.articleFeedbackv5.templates.helpToolTipTrigger );
21272165 $wrapper.find( '.articleFeedbackv5-tooltip-trigger' ).click( function () {
——@@ -2165,9 +2203,41 @@
21662204 } );
21672205 $titlebar.localize( { 'prefix': 'articlefeedbackv5-' } );
21682206
2169 - // Set loaded
2170 - $.articleFeedbackv5.isLoaded = true;
 2207+ // Mark that we have containers
 2208+ $.articleFeedbackv5.hasContainers = true;
 2209+ };
21712210
 2211+ // }}}
 2212+ // {{{ showForm
 2213+
 2214+ /**
 2215+ * Builds the form and loads it into the document
 2216+ */
 2217+ $.articleFeedbackv5.showForm = function () {
 2218+
 2219+ // Build the form
 2220+ var bucket = $.articleFeedbackv5.currentBucket();
 2221+ var $block = bucket.buildForm();
 2222+ if ( 'bindEvents' in bucket ) {
 2223+ bucket.bindEvents( $block );
 2224+ }
 2225+ $block.localize( { 'prefix': 'articlefeedbackv5-' } );
 2226+
 2227+ // Add it to the appropriate container
 2228+ $.articleFeedbackv5.find( '.articleFeedbackv5-ui-inner' )
 2229+ .append( $block );
 2230+
 2231+ // Set the appropriate class on the ui block
 2232+ $.articleFeedbackv5.find( '.articleFeedbackv5-ui' )
 2233+ .addClass( 'articleFeedbackv5-option-' + $.articleFeedbackv5.bucketId )
 2234+ .removeClass( 'articleFeedbackv5-cta-' + $.articleFeedbackv5.ctaId );
 2235+
 2236+ // Set the title
 2237+ if ( 'getTitle' in bucket ) {
 2238+ $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-title' ).html( bucket.getTitle() );
 2239+ $.articleFeedbackv5.$dialog.dialog( 'option', 'title', bucket.getTitle() );
 2240+ }
 2241+
21722242 // Do anything special the bucket requires
21732243 if ( 'afterBuild' in bucket ) {
21742244 bucket.afterBuild();
——@@ -2277,6 +2347,8 @@
22782348 * Shows a CTA
22792349 */
22802350 $.articleFeedbackv5.showCTA = function () {
 2351+
 2352+ // Build the form
22812353 var cta = $.articleFeedbackv5.currentCTA();
22822354 if ( !( 'build' in cta ) ) {
22832355 return;
——@@ -2286,15 +2358,23 @@
22872359 cta.bindEvents( $block );
22882360 }
22892361 $block.localize( { 'prefix': 'articlefeedbackv5-' } );
 2362+
 2363+ // Add it to the appropriate container
 2364+ $.articleFeedbackv5.find( '.articleFeedbackv5-ui-inner' ).empty();
 2365+ $.articleFeedbackv5.find( '.articleFeedbackv5-ui-inner' )
 2366+ .append( $block );
 2367+
 2368+ // Set the appropriate class on the ui block
 2369+ $.articleFeedbackv5.find( '.articleFeedbackv5-ui' )
 2370+ .removeClass( 'articleFeedbackv5-option-' + $.articleFeedbackv5.bucketId )
 2371+ .addClass( 'articleFeedbackv5-cta-' + $.articleFeedbackv5.ctaId );
 2372+
 2373+ // Set the title in both places
22902374 if ( 'getTitle' in cta ) {
2291 - if ( $.articleFeedbackv5.inDialog ) {
2292 - $.articleFeedbackv5.$dialog.dialog( 'option', 'title', cta.getTitle() );
2293 - } else {
2294 - $.articleFeedbackv5.find( '.articleFeedbackv5-title' ).html( cta.getTitle() );
2295 - }
 2375+ $.articleFeedbackv5.$dialog.dialog( 'option', 'title', cta.getTitle() );
 2376+ $.articleFeedbackv5.find( '.articleFeedbackv5-title' ).html( cta.getTitle() );
22962377 }
2297 - $.articleFeedbackv5.find( '.articleFeedbackv5-ui' ).empty();
2298 - $.articleFeedbackv5.find( '.articleFeedbackv5-ui' ).append( $block );
 2378+
22992379 // Add a close button to clear out the panel
23002380 var $close = $( '<a class="articleFeedbackv5-clear-trigger">x</a>' )
23012381 .click( function (e) {
——@@ -2303,6 +2383,10 @@
23042384 } );
23052385 $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-title-wrap .articleFeedbackv5-tooltip-trigger' )
23062386 .before( $close );
 2387+
 2388+ // Reset the panel dimensions
 2389+ $.articleFeedbackv5.setDialogDimensions();
 2390+
23072391 $.articleFeedbackv5.nowShowing = 'cta';
23082392 };
23092393
——@@ -2313,17 +2397,25 @@
23142398 * Clears out the panel
23152399 */
23162400 $.articleFeedbackv5.clear = function () {
2317 -
23182401 $.articleFeedbackv5.isLoaded = false;
23192402 $.articleFeedbackv5.inDialog = false;
23202403 $.articleFeedbackv5.submissionEnabled = false;
23212404 $.articleFeedbackv5.feedbackId = 0;
 2405+ $.articleFeedbackv5.clearContainers();
 2406+ $.articleFeedbackv5.nowShowing = 'none';
 2407+ };
23222408
 2409+ // }}}
 2410+ // {{{ clearContainers
 2411+
 2412+ /**
 2413+ * Wipes the containers from the page
 2414+ */
 2415+ $.articleFeedbackv5.clearContainers = function () {
23232416 $.articleFeedbackv5.$holder.empty();
2324 - $.articleFeedbackv5.$dialog.remove();
2325 -
2326 -
2327 - $.articleFeedbackv5.nowShowing = 'none';
 2417+ if ( $.articleFeedbackv5.$dialog ) {
 2418+ $.articleFeedbackv5.$dialog.remove();
 2419+ }
23282420 };
23292421
23302422 // }}}
——@@ -2366,7 +2458,7 @@
23672459 $err.html( $err.html().replace( "\n", '<br />' ) );
23682460 $.articleFeedbackv5.$toRemove.remove();
23692461 $.articleFeedbackv5.$toRemove = $( [] );
2370 - $.articleFeedbackv5.nowShowing = 'none';
 2462+ $.articleFeedbackv5.nowShowing = 'error';
23712463 };
23722464
23732465 // }}}
——@@ -2518,21 +2610,14 @@
25192611 // $.articleFeedbackv5.clear();
25202612 }
25212613 if ( !$.articleFeedbackv5.isLoaded ) {
2522 - $.articleFeedbackv5.loadForm();
 2614+ $.articleFeedbackv5.load();
25232615 }
25242616 if ( !$.articleFeedbackv5.inDialog ) {
2525 - var w = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-ui' ).width();
2526 - var h = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-ui' ).height();
2527 - var o = $link.offset();
2528 - var x = 'center';
2529 - // var y = o.top - h - 20;
2530 - var y = 'center';
 2617+ $.articleFeedbackv5.setDialogDimensions();
25312618 $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-tooltip' ).hide();
25322619 $inner = $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-ui' ).detach();
25332620 $.articleFeedbackv5.$dialog.append( $inner );
2534 - $.articleFeedbackv5.$dialog.dialog( 'option', 'width', w + 25 );
2535 - $.articleFeedbackv5.$dialog.dialog( 'option', 'height', h + 70 );
2536 - $.articleFeedbackv5.$dialog.dialog( 'option', 'position', [ x, y ] );
 2621+ $.articleFeedbackv5.$dialog.dialog( 'option', 'position', [ 'center', 'center' ] );
25372622 $.articleFeedbackv5.$dialog.dialog( 'open' );
25382623 $.articleFeedbackv5.setLinkId( $link.data( 'linkId' ) );
25392624
——@@ -2564,9 +2649,22 @@
25652650 };
25662651
25672652 // }}}
 2653+ // {{{ setDialogDimensions
25682654
 2655+ /**
 2656+ * Sets the dialog's dimensions
 2657+ */
 2658+ $.articleFeedbackv5.setDialogDimensions = function () {
 2659+ var w = $.articleFeedbackv5.find( '.articleFeedbackv5-ui' ).width();
 2660+ var h = $.articleFeedbackv5.find( '.articleFeedbackv5-ui' ).height();
 2661+ $.articleFeedbackv5.$dialog.dialog( 'option', 'width', w + 25 );
 2662+ $.articleFeedbackv5.$dialog.dialog( 'option', 'height', h + 70 );
 2663+ };
 2664+
25692665 // }}}
25702666
 2667+ // }}}
 2668+
25712669 // }}}
25722670 // {{{ articleFeedbackv5 plugin
25732671

Status & tagging log