r86862 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r86861‎ | r86862 | r86863 >
Date:14:38, 25 April 2011
Author:neilk
Status:ok
Tags:
Comment:
- Better error display, handling for description page errors (partial fix for bug 24758; can remove error'ing uploads, very similar to file page errors)
- Fixes to morphCrossfade to work in more situations (such as typical jquery multiple invocation)
Modified paths:
  • /trunk/extensions/UploadWizard/SpecialUploadWizard.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizard.i18n.php (modified) (history)
  • /trunk/extensions/UploadWizard/UploadWizardHooks.php (modified) (history)
  • /trunk/extensions/UploadWizard/resources/jquery/jquery.morphCrossfade.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizard.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizardDeed.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/mw.UploadWizardDetails.js (modified) (history)
  • /trunk/extensions/UploadWizard/resources/uploadWizard.css (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/UploadWizardHooks.php
——@@ -147,6 +147,8 @@
148148 'mwe-upwiz-transported',
149149 'mwe-upwiz-stashed-upload',
150150 'mwe-upwiz-getting-metadata',
 151+ 'mwe-upwiz-submitting-details',
 152+ 'mwe-upwiz-published',
151153 'mwe-upwiz-failed',
152154 'mwe-upwiz-click-here',
153155 'mwe-upwiz-editing',
Index: trunk/extensions/UploadWizard/UploadWizard.i18n.php
——@@ -78,6 +78,8 @@
7979 'mwe-upwiz-transported' => 'Finished uploading...',
8080 'mwe-upwiz-stashed-upload' => 'OK',
8181 'mwe-upwiz-getting-metadata' => 'Getting file information and previews...',
 82+ 'mwe-upwiz-submitting-details' => 'Submitting details and publishing...',
 83+ 'mwe-upwiz-published' => 'Published!',
8284 'mwe-upwiz-failed' => 'Failed.',
8385 'mwe-upwiz-click-here' => 'Click here to select a file',
8486 'mwe-upwiz-editing' => 'editing...',
Index: trunk/extensions/UploadWizard/resources/jquery/jquery.morphCrossfade.js
——@@ -62,7 +62,10 @@
6363
6464 // should achieve the same result as crossfade( this.children().first() ) but without
6565 // animation etc.
66 - this.morphCrossfade( this.children().first(), 0 );
 66+ $j.each( this, function( i, container ) {
 67+ var $container = $j( container );
 68+ $container.morphCrossfade( $container.children().first(), 0 );
 69+ } );
6770
6871 return this;
6972 };
——@@ -73,38 +76,45 @@
7477 * @param speed (optional) how fast to crossfade, in milliseconds
7578 */
7679 $.fn.morphCrossfade = function( newPanelSelector, speed ) {
77 - var container = this;
 80+ var $containers = this;
7881 if ( typeof speed === 'undefined' ) {
7982 speed = 400;
8083 }
8184
82 - container.css( { 'overflow' : 'hidden' } );
 85+ $containers.css( { 'overflow' : 'hidden' } );
8386
84 - $oldPanel = $( container.data( 'crossfadeDisplay' ) );
85 - if ( $oldPanel ) {
86 - // remove auto setting of height from container, and
87 - // make doubly sure that the container height is equal to oldPanel
88 - container.css( { height: $oldPanel.outerHeight() } );
89 - // take it out of the flow
90 - $oldPanel.css( { position: 'absolute' } );
91 - // fade WITHOUT hiding when opacity = 0
92 - $oldPanel.animate( { opacity: 0 }, speed, 'linear', function() {
93 - $oldPanel.css( { visibility: 'hidden'} );
94 - } );
95 - }
96 - container.data( 'crossfadeDisplay', newPanelSelector );
 87+
 88+ $j.each( $containers, function( i, container ) {
 89+ var $container = $j( container );
 90+ var $oldPanel = $( $container.data( 'crossfadeDisplay' ) );
 91+ var $newPanel = ( typeof newPanelSelector === 'string' ) ? $container.find( newPanelSelector ) : $j( newPanelSelector );
9792
98 - var $newPanel = $( newPanelSelector );
99 - $newPanel.css( { visibility: 'visible' } );
100 - container.animate( { height: $newPanel.outerHeight() }, speed, 'linear', function() {
101 - // we place it back into the flow, in case its size changes.
102 - $newPanel.css( { position: 'relative' } );
103 - // and allow the container to grow with it.
104 - container.css( { height : 'auto' } );
 93+ if ( $oldPanel.get(0) !== $newPanel.get(0) ) {
 94+ if ( $oldPanel ) {
 95+ // remove auto setting of height from container, and
 96+ // make doubly sure that the container height is equal to oldPanel
 97+ $container.css( { height: $oldPanel.outerHeight() } );
 98+ // take it out of the flow
 99+ $oldPanel.css( { position: 'absolute' } );
 100+ // fade WITHOUT hiding when opacity = 0
 101+ $oldPanel.animate( { opacity: 0 }, speed, 'linear', function() {
 102+ $oldPanel.css( { visibility: 'hidden'} );
 103+ } );
 104+ }
 105+ $container.data( 'crossfadeDisplay', $newPanel );
 106+
 107+ $newPanel.css( { visibility: 'visible' } );
 108+ $container.animate( { height: $newPanel.outerHeight() }, speed, 'linear', function() {
 109+ // we place it back into the flow, in case its size changes.
 110+ $newPanel.css( { position: 'relative' } );
 111+ // and allow the container to grow with it.
 112+ $container.css( { height : 'auto' } );
 113+ } );
 114+ $newPanel.animate( { opacity: 1 }, speed );
 115+ }
105116 } );
106 - $newPanel.animate( { opacity: 1 }, speed );
107117
108 - return container;
 118+ return this;
109119 };
110120
111121 } )( jQuery );
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizardDeed.js
——@@ -116,7 +116,7 @@
117117 .msg( 'mwe-upwiz-license-show-all' )
118118 .click( function() {
119119 _this.formValidator.resetForm();
120 - if ( $crossfader.data( 'crossfadeDisplay' ) === $customDiv ) {
 120+ if ( $crossfader.data( 'crossfadeDisplay' ).get(0) === $customDiv.get(0) ) {
121121 _this.licenseInput.setDefaultValues();
122122 $crossfader.morphCrossfade( $standardDiv );
123123 $j( this ).msg( 'mwe-upwiz-license-show-all' );
Index: trunk/extensions/UploadWizard/resources/uploadWizard.css
——@@ -704,3 +704,7 @@
705705 .ui-dialog .mwe-upwiz-lightbox {
706706 padding: 0;
707707 }
 708+
 709+.mwe-upwiz-details-texts {
 710+ padding: 0.5em;
 711+}
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizard.js
——@@ -633,8 +633,9 @@
634634 } );
635635
636636 $j( '#mwe-upwiz-stepdiv-file .mwe-upwiz-buttons .mwe-upwiz-button-next' ).click( function() {
637 - _this.removeErrorUploads();
638 - _this.prepareAndMoveToDeeds();
 637+ _this.removeErrorUploads( function() {
 638+ _this.prepareAndMoveToDeeds();
 639+ } );
639640 } );
640641 $j ( '#mwe-upwiz-stepdiv-file .mwe-upwiz-buttons .mwe-upwiz-button-retry' ).click( function() {
641642 _this.hideFileEndButtons();
——@@ -671,23 +672,39 @@
672673
673674
674675 // DETAILS div
 676+ var finalizeDetails = function() {
 677+ if ( mw.isDefined( _this.allowCloseWindow ) ) {
 678+ _this.allowCloseWindow();
 679+ }
 680+ _this.prefillThanksPage();
 681+ _this.moveToStep( 'thanks' );
 682+ };
675683
676 - $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-button-next' )
 684+ var startDetails = function() {
 685+ $j( '.mwe-upwiz-hint' ).each( function(i) { $j( this ).tipsy( 'hide' ); } ); // close tipsy help balloons
 686+ if ( _this.detailsValid() ) {
 687+ _this.hideDetailsEndButtons();
 688+ _this.detailsSubmit( function() {
 689+ _this.showNext( 'details', 'complete', finalizeDetails );
 690+ } );
 691+ }
 692+ };
 693+
 694+ $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-file-next-some-failed' ).hide();
 695+ $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-file-next-all-failed' ).hide();
 696+
 697+ $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-start-next .mwe-upwiz-button-next' )
 698+ .click( startDetails );
 699+
 700+ $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-buttons .mwe-upwiz-button-next-despite-failures' )
677701 .click( function() {
678 - $j( '.mwe-upwiz-hint' ).each( function(i) { $j( this ).tipsy( 'hide' ); } ); // close tipsy help balloons
679 - if ( _this.detailsValid() ) {
680 - _this.detailsSubmit( function() {
681 - if ( mw.isDefined( _this.allowCloseWindow ) ) {
682 - _this.allowCloseWindow();
683 - }
684 - _this.prefillThanksPage();
685 - _this.moveToStep( 'thanks' );
686 - } );
687 - }
 702+ _this.removeErrorUploads( finalizeDetails );
688703 } );
 704+
 705+ $j ( '#mwe-upwiz-stepdiv-details .mwe-upwiz-buttons .mwe-upwiz-button-retry' )
 706+ .click( startDetails );
689707
690708
691 -
692709 // WIZARD
693710
694711 // check to see if the the skip tutorial cookie is set
——@@ -878,10 +895,13 @@
879896 * Hide the button choices at the end of the file step.
880897 */
881898 hideFileEndButtons: function() {
882 - $j( '#mwe-upwiz-stepdiv .mwe-upwiz-buttons' ).hide();
883899 $j( '#mwe-upwiz-stepdiv-file .mwe-upwiz-buttons .mwe-upwiz-file-endchoice' ).hide();
884900 },
885901
 902+ hideDetailsEndButtons: function() {
 903+ $j( '#mwe-upwiz-stepdiv-details .mwe-upwiz-buttons .mwe-upwiz-file-endchoice' ).hide();
 904+ },
 905+
886906 /**
887907 * This is useful to clean out unused upload file inputs if the user hits GO.
888908 * We are using a second array to iterate, because we will be splicing the main one, _this.uploads
——@@ -894,11 +914,13 @@
895915
896916 /**
897917 * Clear out uploads that are in error mode, perhaps before proceeding to the next step
 918+ * @param {Function} to be called when done
898919 */
899 - removeErrorUploads: function() {
 920+ removeErrorUploads: function( endCallback ) {
900921 this.removeMatchingUploads( function( upload ) {
901922 return upload.state === 'error';
902923 } );
 924+ endCallback();
903925 },
904926
905927
——@@ -1016,7 +1038,7 @@
10171039 },
10181040 function() {
10191041 $j().notify( gM( 'mwe-upwiz-files-complete' ) );
1020 - _this.showFileNext();
 1042+ _this.showNext( 'file', 'stashed' );
10211043 }
10221044 );
10231045 },
——@@ -1030,29 +1052,26 @@
10311053 * 4) All failed -- have to retry, no other option
10321054 * In principle there could be other configurations, like having the uploads not all in error or stashed state, but
10331055 * we trust that this hasn't happened.
 1056+ *
 1057+ * @param {String} step that we are on
 1058+ * @param {String} desired state to proceed (other state is assumed to be 'error')
10341059 */
1035 - showFileNext: function() {
1036 - if ( this.uploads.length === 0 ) {
1037 - this.updateFileCounts();
1038 - $j( '#mwe-upwiz-progress' ).hide();
1039 - $j( '#mwe-upwiz-upload-ctrls' ).show();
1040 - $j( '#mwe-upwiz-add-file' ).show();
1041 - this.moveToStep( 'file' );
1042 - return;
1043 - }
 1060+ showNext: function( step, desiredState, allOkCallback ) {
10441061 var errorCount = 0;
1045 - var stashedCount = 0;
 1062+ var okCount = 0;
10461063 $j.each( this.uploads, function( i, upload ) {
10471064 if ( upload.state === 'error' ) {
10481065 errorCount++;
1049 - } else if ( upload.state === 'stashed' ) {
1050 - stashedCount++;
 1066+ } else if ( upload.state === desiredState ) {
 1067+ okCount++;
10511068 } else {
10521069 mw.log( "mw.UploadWizardUpload::showFileNext> upload " + i + " not in appropriate state for filenext: " + upload.state );
10531070 }
10541071 } );
10551072 var selector = null;
1056 - if ( stashedCount === this.uploads.length ) {
 1073+ var allOk = false;
 1074+ if ( okCount === this.uploads.length ) {
 1075+ allOk = true;
10571076 selector = '.mwe-upwiz-file-next-all-ok';
10581077 } else if ( errorCount === this.uploads.length ) {
10591078 selector = '.mwe-upwiz-file-next-all-failed';
——@@ -1060,9 +1079,11 @@
10611080 selector = '.mwe-upwiz-file-next-some-failed';
10621081 }
10631082
1064 - // perhaps the button should slide down?
1065 - $j( '#mwe-upwiz-stepdiv-file .mwe-upwiz-buttons' ).show().find( selector ).show();
1066 -
 1083+ if ( allOk && mw.isDefined( allOkCallback ) ) {
 1084+ allOkCallback();
 1085+ } else {
 1086+ $j( '#mwe-upwiz-stepdiv-' + step + ' .mwe-upwiz-buttons' ).show().find( selector ).show();
 1087+ }
10671088 },
10681089
10691090 /**
——@@ -1169,35 +1190,31 @@
11701191 */
11711192 detailsSubmit: function( endCallback ) {
11721193 var _this = this;
1173 - // some details blocks cannot be submitted (for instance, identical file hash)
1174 - _this.removeBlockedDetails();
11751194
1176 - // remove ability to edit details
1177 - $j.each( _this.uploads, function( i, upload ) {
1178 - upload.details.div.mask();
 1195+ $j.each( _this.uploads, function( i, upload ) {
 1196+ $j( upload.details.submittingDiv )
 1197+ .find( '.mwe-upwiz-visible-file-filename-text' )
 1198+ .html( upload.title.getMain() );
11791199 } );
11801200
 1201+ // remove ability to edit details
 1202+ $j( '#mwe-upwiz-stepdiv-details' )
 1203+ .find( '.mwe-upwiz-data' )
 1204+ .morphCrossfade( '.mwe-upwiz-submitting' );
 1205+
11811206 // add the upload progress bar, with ETA
11821207 // add in the upload count
11831208 _this.makeTransitioner(
11841209 'details',
11851210 [ 'submitting-details' ],
1186 - [ 'complete' ],
 1211+ [ 'error', 'complete' ],
11871212 function( upload ) {
11881213 upload.details.submit();
11891214 },
1190 - endCallback /* called when all uploads are "complete" */
 1215+ endCallback /* called when all uploads are in a valid end state */
11911216 );
11921217 },
11931218
1194 - /**
1195 - * Removes(?) details that we can't edit for whatever reason -- might just advance them to a different state?
1196 - */
1197 - removeBlockedDetails: function() {
1198 - // TODO
1199 - },
1200 -
1201 -
12021219 prefillThanksPage: function() {
12031220 var _this = this;
12041221
——@@ -1311,7 +1328,7 @@
13121329 {
13131330 text: gM( 'mwe-upwiz-cancel', uploads.length ),
13141331 click: function() {
1315 - $j( this ).dialog( 'close' )
 1332+ $j( this ).dialog( 'close' );
13161333 }
13171334 }
13181335 ];
Index: trunk/extensions/UploadWizard/resources/mw.UploadWizardDetails.js
——@@ -174,9 +174,19 @@
175175 moreDetailsDiv
176176 );
177177
 178+ _this.submittingDiv = $j( '<div></div>' ).addClass( 'mwe-upwiz-submitting' )
 179+ .append(
 180+ $j( '<div></div>' ).addClass( 'mwe-upwiz-file-indicator' ),
 181+ $j( '<div></div>' ).addClass( 'mwe-upwiz-details-texts' ).append(
 182+ $j( '<div></div>' ).addClass( 'mwe-upwiz-visible-file-filename-text' ),
 183+ $j( '<div></div>' ).addClass( 'mwe-upwiz-file-status-line' )
 184+ )
 185+ );
 186+
178187 $j( _this.dataDiv ).append(
179 - _this.$form
180 - );
 188+ _this.$form,
 189+ _this.submittingDiv
 190+ ).morphCrossfader();
181191
182192 $j( _this.div ).append(
183193 _this.thumbnailDiv,
——@@ -657,6 +667,7 @@
658668 var _this = this;
659669
660670 _this.upload.state = 'submitting-details';
 671+ _this.setStatus( gM( 'mwe-upwiz-submitting-details' ) );
661672 _this.showIndicator( 'progress' );
662673
663674 // XXX check state of details for okayness ( license selected, at least one desc, sane filename )
——@@ -672,6 +683,7 @@
673684 };
674685
675686 var err = function( code, info ) {
 687+ _this.upload.state = 'error';
676688 _this.showError( code, info );
677689 };
678690
——@@ -682,7 +694,7 @@
683695 _this.upload.state = 'complete';
684696 _this.showIndicator( 'uploaded' );
685697 } else {
686 - _this.showError( 'details-info-missing', result );
 698+ err( 'details-info-missing', result );
687699 }
688700 };
689701
——@@ -701,11 +713,11 @@
702714 },
703715
704716 setStatus: function( s ) {
705 - this.div.data( 'statusLine' ).html( s ).show();
 717+ this.div.find( '.mwe-upwiz-file-status-line' ).html( s ).show();
706718 },
707719
708720 showIndicator: function( statusStr ) {
709 - this.div.data( 'indicator' )
 721+ this.div.find( '.mwe-upwiz-file-indicator' )
710722 .show()
711723 .removeClass( 'mwe-upwiz-status-progress mwe-upwiz-status-error mwe-upwiz-status-uploaded' )
712724 .addClass( 'mwe-upwiz-status-' + statusStr );
Index: trunk/extensions/UploadWizard/SpecialUploadWizard.php
——@@ -265,7 +265,18 @@
266266 . '<div class="mwe-upwiz-stepdiv" id="mwe-upwiz-stepdiv-details" style="display:none;">'
267267 . '<div id="mwe-upwiz-macro-files" class="mwe-upwiz-filled-filelist ui-corner-all"></div>'
268268 . '<div class="mwe-upwiz-buttons">'
269 - . '<button class="mwe-upwiz-button-next">' . wfMsg( "mwe-upwiz-next-details" ) . '</button>'
 269+ . '<div class="mwe-upwiz-start-next mwe-upwiz-file-endchoice">'
 270+ . '<button class="mwe-upwiz-button-next">' . wfMsg( "mwe-upwiz-next-details" ) . '</button>'
 271+ . '</div>'
 272+ . '<div class="mwe-upwiz-file-next-some-failed mwe-upwiz-file-endchoice">'
 273+ . wfMsg( "mwe-upwiz-file-some-failed" )
 274+ . '<button class="mwe-upwiz-button-retry">' . wfMsg( "mwe-upwiz-file-retry" ) . '</button>'
 275+ . '<button class="mwe-upwiz-button-next-despite-failures">' . wfMsg( "mwe-upwiz-next-file-despite-failures" ) . '</button>'
 276+ . '</div>'
 277+ . '<div class="mwe-upwiz-file-next-all-failed mwe-upwiz-file-endchoice">'
 278+ . wfMsg( "mwe-upwiz-file-all-failed" )
 279+ . '<button class="mwe-upwiz-button-retry"> ' . wfMsg( "mwe-upwiz-file-retry" ) . '</button>'
 280+ . '</div>'
270281 . '</div>'
271282 . '</div>'
272283

Status & tagging log