r61513 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r61512‎ | r61513 | r61514 >
Date:09:39, 26 January 2010
Author:shmichael
Status:deferred (Comments)
Tags:
Comment:
* Moved authority of search filters to searchLibs.
* Specifically, new class kalturaFilters handles filter definitions and creates
an HTML representation of them for embedding within the page.
* Multiple minor and syntax corrections
* Filters are now fully controlled by showResults within the RemoteSearchDriver
Modified paths:
  • /branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/mw.RemoteSearchDriver.js (modified) (history)
  • /branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/searchLibs/kalturaSearch.js (modified) (history)
  • /branches/js2-work/phase3/js/mwEmbed/skins/common/common.css (modified) (history)

Diff [purge]

Index: branches/js2-work/phase3/js/mwEmbed/skins/common/common.css
——@@ -96,9 +96,7 @@
9797 }
9898 #rsd_filters_container {
9999 float:left;
100 - padding-top:12px;
101100 width:150px;
102 - height: 100%;
103101 }
104102
105103 .ui-filter-title {
——@@ -451,4 +449,4 @@
452450
453451 .interface_wrap{
454452 position:relative;
455 -}
\ No newline at end of file
 453+}
Index: branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/searchLibs/kalturaSearch.js
——@@ -2,6 +2,95 @@
33 * Kaltura aggregated search:
44 */
55
 6+var kalturaFilters = function ( options ) {
 7+ return this.init( options );
 8+}
 9+
 10+kalturaFilters.prototype = {
 11+
 12+ init: function( options ) {
 13+ var mediaTypes = {
 14+ video: 'Videos',
 15+ image: 'Images'
 16+ };
 17+
 18+ var providers = {
 19+ wiki_commons: 'Wikipedia Commons',
 20+ archive_org: 'The Internet Archive',
 21+ metavid: 'Metavid',
 22+ flickr: 'Flickr'
 23+ };
 24+
 25+ this.filterList = {
 26+ media: { title: 'Media', options: mediaTypes },
 27+ providers: { title: 'Providers', options: providers }
 28+ };
 29+
 30+ return this;
 31+ },
 32+
 33+
 34+ /**
 35+ * Create an HTML representation of the available search filters and append
 36+ * them to the given element.
 37+ *
 38+ * @param {jQuery element} The base element to which HTML items should be
 39+ * appended.
 40+ */
 41+
 42+ getHTML: function() {
 43+ var _this = this;
 44+ mw.log( 'f: populateFilterContainer ' );
 45+
 46+ $filtersContainer = $j( '<div />' );
 47+
 48+ for (filter in this.filterList) {
 49+ $filtersContainer.append(
 50+ this.getFilterBox( 'rsd_' + filter + '_filter',
 51+ this.filterList[ filter ].title,
 52+ this.filterList[ filter ].options ));
 53+ }
 54+
 55+ return $filtersContainer;
 56+ },
 57+
 58+ /**
 59+ * Creates a single filter box with given selection options
 60+ *
 61+ * @id {string} unique id for this filter box an residing elements
 62+ * @title {string} title of the filter box
 63+ * @options {array} array of strings describing the options in the filter box
 64+ *
 65+ */
 66+
 67+ getFilterBox: function( id, title, filterOptions ) {
 68+ $box = $j( '<div />' ).addClass( 'ui-filter-box' ).attr({
 69+ 'id': id
 70+ });
 71+
 72+ $title = $j( '<div />' ).addClass( 'ui-filter-title' ).text( title );
 73+ $box.append( $title );
 74+
 75+ for (filterID in filterOptions) {
 76+ $option = $j( '<div />' ).addClass( 'ui-filter-option' ).text( filterOptions[ filterID ] );
 77+
 78+ $checkbox = $j( '<input />' )
 79+ .attr( {
 80+ type: 'checkbox',
 81+ name: id + '_' + title + '_' + filterID,
 82+ value: filterID,
 83+ checked: true
 84+ } );
 85+
 86+ $option.prepend( $checkbox );
 87+ $box.append( $option );
 88+ }
 89+
 90+ return $box;
 91+ },
 92+
 93+};
 94+
695 var kalturaSearch = function ( options ) {
796 return this.init( options );
897 }
——@@ -12,12 +101,13 @@
13102 searchLibs: { },
14103
15104 /**
16 - * Initialize the flickr Search with provided options
 105+ * Initialize the Search with provided options
17106 *
18107 * @param {Object} options Initial options for the kalturaSearch class
19108 */
20109 init:function( options ) {
21110 this.options = options;
 111+ this.filters = new kalturaFilters( options );
22112 var baseSearch = new baseRemoteSearch( options );
23113 for ( var i in baseSearch ) {
24114 if ( typeof this[i] == 'undefined' ) {
——@@ -26,6 +116,8 @@
27117 this['parent_' + i] = baseSearch[i];
28118 }
29119 }
 120+
 121+ return this;
30122 },
31123
32124 /**
Index: branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/mw.RemoteSearchDriver.js
——@@ -255,13 +255,7 @@
256256 'lib': 'kaltura',
257257 'resource_prefix' : '',
258258 'tab_image':false,
259 - show_filters: true,
260 - aggregated_providers:{
261 - 'Wikipedia Commons': 'wiki_commons',
262 - 'The Internet Archive': 'archive_org',
263 - 'Metavid': 'metavid',
264 - 'Flickr': 'flickr'
265 - }
 259+ disable_filters: true
266260 },
267261
268262 /**
——@@ -668,7 +662,7 @@
669663 this.current_provider == 'upload' )
670664 {
671665 $j( '#rsd_q' ).val( query );
672 - _this.UpdateResults();
 666+ _this.updateResults();
673667 }
674668 // $j(_this.target_container).dialog("open");
675669 $j( _this.target_container ).parents( '.ui-dialog' ).fadeIn( 'slow' );
——@@ -807,11 +801,6 @@
808802 var controlContainer = this.createControlContainer();
809803
810804 mainContainer.append( controlContainer );
811 -
812 - this.$filtersContainer = $j('<form />').hide()
813 - .attr({
814 - id: "rsd_filters_container"
815 - });
816805
817806 this.$resultsContainer = $j('<div />').attr({
818807 id: "rsd_results_container"
——@@ -822,7 +811,7 @@
823812
824813 // Run the default search:
825814 if ( this.getDefaultQuery() )
826 - this.UpdateResults();
 815+ this.updateResults();
827816
828817 // Add bindings
829818 $j( '#mso_selprovider,#mso_selprovider_close' )
——@@ -846,7 +835,7 @@
847836 $j( '#rsd_form' )
848837 .unbind()
849838 .submit( function() {
850 - _this.UpdateResults();
 839+ _this.updateResults();
851840 // Don't submit the form
852841 return false;
853842 } );
——@@ -875,7 +864,7 @@
876865 .addClass( 'rsd_search_button' )
877866 .buttonHover()
878867 .click(function (){
879 - _this.UpdateResults( _this.current_provider, true );
 868+ _this.updateResults( _this.current_provider, true );
880869 });
881870 var $searchBox = $j( '<input />' ).addClass( 'ui-widget-content ui-corner-all' ).attr({
882871 type: "text",
——@@ -917,7 +906,7 @@
918907 $j( this ).addClass( 'ui-selected' );
919908 _this.current_provider = $j( this ).attr( "name" );
920909 // Update the search results on provider selection
921 - _this.UpdateResults( _this.current_provider, true );
 910+ _this.updateResults( _this.current_provider, true );
922911 });
923912
924913 var $listItem = $j( '<li />' );
——@@ -935,7 +924,7 @@
936925 .addClass("rsd_upload_button")
937926 .click(function(){
938927 _this.current_provider = 'upload';
939 - _this.UpdateUploadResults( );
 928+ _this.updateUploadResults( );
940929 });
941930 $searchForm.append( $uploadButton );
942931 /*
——@@ -954,8 +943,8 @@
955944 /**
956945 * Shows the upload tab loader and issues a call to showUploadForm
957946 */
958 - UpdateUploadResults: function() {
959 - mw.log( "UpdateUploadResults::" );
 947+ updateUploadResults: function() {
 948+ mw.log( "updateUploadResults::" );
960949 var _this = this;
961950 // set it to loading:
962951 this.$resultsContainer.loadingSpinner();
——@@ -1046,12 +1035,11 @@
10471036 /**
10481037 * Refresh the results container ( based on current_provider var )
10491038 */
1050 - UpdateResults: function() {
1051 - this.$filtersContainer.hide();
 1039+ updateResults: function() {
10521040 if ( this.current_provider == 'upload' ) {
1053 - this.UpdateUploadResults();
 1041+ this.updateUploadResults();
10541042 } else {
1055 - this.UpdateSearchResults( this.current_provider, false );
 1043+ this.updateSearchResults( this.current_provider, false );
10561044 }
10571045 },
10581046
——@@ -1061,8 +1049,8 @@
10621050 * @param {String} providerName name of the content provider
10631051 * @param {Bollean} resetPaging if the pagging should be reset
10641052 */
1065 - UpdateSearchResults: function( providerName, resetPaging ) {
1066 - mw.log( "f:UpdateSearchResults::" + providerName );
 1053+ updateSearchResults: function( providerName, resetPaging ) {
 1054+ mw.log( "f:updateSearchResults::" + providerName );
10671055
10681056 var draw_direct_flag = true;
10691057 var provider = this.content_providers[ providerName ];
——@@ -1098,7 +1086,7 @@
10991087 this.$resultsContainer.html( mw.loading_spinner() );
11001088
11011089 // Make sure the search library is loaded and issue the search request
1102 - this.PerformProviderSearch( provider );
 1090+ this.performProviderSearch( provider );
11031091 },
11041092
11051093 /*
——@@ -1194,74 +1182,22 @@
11951183 } );
11961184 },
11971185
1198 -
1199 - createSearchFilters: function( current_provider ) {
1200 - var _this = this;
1201 - mw.log( 'f: CreateSearchFilters ' );
1202 -
1203 - this.CreateFilterBox( 'rsd_media_type_filter',
1204 - 'Media',
1205 - { Video: 'video',
1206 - Image: 'image',
1207 - Audio: 'audio' } );
1208 -
1209 - this.CreateFilterBox( 'rsd_provider_filter',
1210 - 'Provider',
1211 - current_provider.aggregated_providers );
1212 -
1213 - },
1214 -
12151186 /**
1216 - * Creates a single filter box with given options and appends it to the filter container.
1217 - *
1218 - * @id {string} unique id for this filter box an residing elements
1219 - * @title {string} title of the filter box
1220 - * @options {array} array of strings describing the options in the filter box
1221 - *
1222 - */
1223 -
1224 - CreateFilterBox: function( id, title, options ) {
1225 - $box = $j( '<div />' ).addClass( 'ui-filter-box' ).attr({
1226 - 'id': id
1227 - });
1228 -
1229 - $title = $j( '<div />' ).addClass( 'ui-filter-title' ).text( title );
1230 - $box.append( $title );
1231 -
1232 - for (option in options) {
1233 - $option = $j( '<div />' ).addClass( 'ui-filter-option' ).text( option );
1234 -
1235 - $checkbox = $j( '<input />' )
1236 - .attr( {
1237 - type: 'checkbox',
1238 - name: id + '_' + title + '_' + options.option,
1239 - value: options.option,
1240 - checked: true
1241 - } );
1242 -
1243 - $option.prepend( $checkbox );
1244 - $box.append( $option );
1245 - }
1246 -
1247 - this.$filtersContainer.append($box);
1248 - },
1249 -
1250 - /**
12511187 * Performs the search for a given content provider
12521188 *
12531189 * Calls showResults once search results are ready
12541190 *
12551191 * @param {Object} provider the provider to be searched.
12561192 */
1257 - PerformProviderSearch: function( provider ) {
 1193+ performProviderSearch: function( provider ) {
12581194 var _this = this;
1259 - mw.log( 'f: PerformProviderSearch ' );
 1195+ mw.log( 'f: performProviderSearch ' );
12601196 // First check if we should even run the search at all (can we import / insert
12611197 // into the page? )
12621198 if ( !this.isProviderLocal( provider ) && this.import_url_mode == 'autodetect' ) {
12631199 // provider is not local check if we can support the import mode:
12641200 this.checkForCopyURLSupport( function() {
1265 - _this.PerformProviderSearch( provider );
 1201+ _this.performProviderSearch( provider );
12661202 } );
12671203 return false;
12681204 } else if ( !this.isProviderLocal( provider ) && this.import_url_mode == 'none' ) {
——@@ -1378,9 +1314,10 @@
13791315 $resultsContainer.empty().append( this.createResultsHeader() )
13801316
13811317 // Enable search filters, if the provider supports them.
1382 - if ( provider.show_filters ) {
1383 - // this.$filtersContainer.show();
1384 - this.createSearchFilters( provider );
 1318+ if ( provider.sObj.filters && !(provider.disable_filters) ) {
 1319+ $resultsContainer.append( provider.sObj.filters.getHTML().attr ({
 1320+ id: 'rsd_filters_container'
 1321+ }));
13851322 }
13861323 }
13871324
——@@ -2861,7 +2798,7 @@
28622799 provider.offset -= provider.limit;
28632800 if ( provider.offset < 0 )
28642801 provider.offset = 0;
2865 - _this.UpdateResults();
 2802+ _this.updateResults();
28662803 } );
28672804 $pagingControl.prepend( $prevLink );
28682805 }
——@@ -2877,7 +2814,7 @@
28782815 .text( nextLinkText )
28792816 .click( function() {
28802817 provider.offset += provider.limit;
2881 - _this.UpdateResults();
 2818+ _this.updateResults();
28822819 } );
28832820 $pagingControl.append( $nextLink );
28842821 }
——@@ -2893,10 +2830,10 @@
28942831 mw.log( 'select tab: ' + provider_id );
28952832 this.current_provider = provider_id;
28962833 if ( this.current_provider == 'upload' ) {
2897 - this.UpdateUploadResults();
 2834+ this.updateUploadResults();
28982835 } else {
28992836 // update the search results:
2900 - this.UpdateResults();
 2837+ this.updateResults();
29012838 }
29022839 },
29032840

Follow-up revisions

RevisionCommit summaryAuthorDate
r61518* followup to r61513...shmichael15:44, 26 January 2010
r61743* Addressed issues in r61513 and follow up to r61518 ....shmichael13:43, 31 January 2010

Comments

#Comment by Spamsplace (talk | contribs)   09:44, 26 January 2010

This is a followup to http://www.mediawiki.org/wiki/Special:Code/MediaWiki/61466 . How do I tag it as such?

#Comment by Mdale (talk | contribs)   17:31, 26 January 2010

You should mention the revision in the commit .. like you did with r61518 then reply to the comment in the older commit r61466 stating that you addressed the issue. And or it will show up in the follow-up revisions because you mention the revision in your commit msg.

#Comment by Spamsplace (talk | contribs)   13:47, 31 January 2010

Beginner's question - how do I revise my commit message? A google search indicates svn:log can only be revised by an administrator.

#Comment by Mdale (talk | contribs)   17:51, 26 January 2010
  • you should define configuration without negatives ie "disable_filters: true" should be what you had before 'enable_filters' ... ( looks like it was removed completely later on anyway )
  • we should have a loader spinner when the user changes the filter set.
  • might want to overflow-auto on the rsd_filters_container for very small height display of the add-media-wizard
#Comment by Spamsplace (talk | contribs)   18:08, 26 January 2010

The reason I defined a negative was to allow this setting to be omitted altogether without confusing consequences.

Agreed on other 2 issues.

Status & tagging log