r61466 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r61465‎ | r61466 | r61467 >
Date:16:14, 24 January 2010
Author:shmichael
Status:resolved (Comments)
Tags:
Comment:
* Implemented front-end of search filters.
Modified paths:
  • /branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/mw.RemoteSearchDriver.js (modified) (history)
  • /branches/js2-work/phase3/js/mwEmbed/skins/mvpcf/styles.css (modified) (history)

Diff [purge]

Index: branches/js2-work/phase3/js/mwEmbed/skins/mvpcf/styles.css
@@ -315,6 +315,12 @@
316316 left: 3px;
317317 font-size: x-small;
318318 }
 319+#rsd_filters_container {
 320+ float:left;
 321+ padding-top:12px;
 322+ width:150px;
 323+ height: 100%;
 324+}
319325 .rsd_import_button {
320326 mergin-left: 5px;
321327 }
@@ -484,6 +490,13 @@
485491 zoom: 1;
486492 }
487493
 494+.ui-filter-title {
 495+ font-weight: bold;
 496+ margin-top: 15px;
 497+ padding-left: 10px;
 498+}
 499+
 500+
488501 .ui-progressbar-value{
489502 background-image: url('images/pbar-ani.gif');
490503 }
Index: branches/js2-work/phase3/js/mwEmbed/modules/AddMedia/mw.RemoteSearchDriver.js
@@ -253,7 +253,13 @@
254254 'api_url': 'http://kaldev.kaltura.com/michael/aggregator.php',
255255 'lib': 'kaltura',
256256 'resource_prefix' : '',
257 - 'tab_image':false
 257+ 'tab_image':false,
 258+ show_filters: true,
 259+ aggregated_providers:
 260+ { 'Wikipedia Commons': 'wiki_commons',
 261+ 'The Internet Archive': 'archive_org',
 262+ 'Metavid': 'metavid',
 263+ 'Flickr': 'flickr' }
258264 },
259265
260266 /**
@@ -474,7 +480,7 @@
475481 // Set up the local API upload URL
476482 if ( _this.upload_api_target == 'local' ) {
477483 if ( ! _this.local_wiki_api_url ) {
478 - $j( '#rsd_results_container' ).html( gM( 'rsd_config_error', 'missing_local_api_url' ) );
 484+ this.$resultsContainer.html( gM( 'rsd_config_error', 'missing_local_api_url' ) );
479485 return false;
480486 } else {
481487 _this.upload_api_target = _this.local_wiki_api_url;
@@ -655,7 +661,7 @@
656662 var query = _this.getDefaultQuery();
657663 if ( query != $j( '#rsd_q' ).val() ) {
658664 $j( '#rsd_q' ).val( query );
659 - _this.showCurrentTab();
 665+ _this.UpdateResults();
660666 }
661667 // $j(_this.target_container).dialog("open");
662668 $j( _this.target_container ).parents( '.ui-dialog' ).fadeIn( 'slow' );
@@ -776,9 +782,6 @@
777783 $j( window ).resize( function() {
778784 $j( _this.target_container ).dialogFitWindow();
779785 } );
780 -
781 - // Add cancel callback and updated button with icon
782 - _this.onCancelClipEdit();
783786 },
784787
785788 /*
@@ -795,15 +798,21 @@
796799
797800 mainContainer.append( controlContainer );
798801
 802+ this.$filtersContainer = $j('<form />').hide()
 803+ .attr({
 804+ id: "rsd_filters_container"
 805+ });
 806+
799807 this.$resultsContainer = $j('<div />').attr({
800 - id: "rsd_results_container"
801 - });
 808+ id: "rsd_results_container"
 809+ });
802810
 811+ mainContainer.append( this.$filtersContainer );
803812 mainContainer.append( this.$resultsContainer );
804813
805814 // run the default search:
806815 if ( this.getDefaultQuery() )
807 - this.showCurrentTab();
 816+ this.UpdateResults();
808817
809818 // Add bindings
810819 $j( '#mso_selprovider,#mso_selprovider_close' )
@@ -827,7 +836,7 @@
828837 $j( '#rsd_form' )
829838 .unbind()
830839 .submit( function() {
831 - _this.showCurrentTab();
 840+ _this.UpdateResults();
832841 // Don't submit the form
833842 return false;
834843 } );
@@ -853,7 +862,7 @@
854863 .addClass( 'rsd_search_button' )
855864 .buttonHover()
856865 .click(function (){
857 - _this.showSearchTab( _this.current_provider, true );
 866+ _this.UpdateResults( _this.current_provider, true );
858867 });
859868 var $searchBox = $j( '<input />' ).addClass( 'ui-widget-content ui-corner-all' ).attr({
860869 type: "text",
@@ -895,7 +904,7 @@
896905 $j( this ).addClass( 'ui-selected' );
897906 _this.current_provider = $j( this ).attr( "name" );
898907 // Update the search results on provider selection
899 - _this.showSearchTab( _this.current_provider, true );
 908+ _this.UpdateResults( _this.current_provider, true );
900909 });
901910
902911 var $listItem = $j( '<li />' );
@@ -913,7 +922,7 @@
914923 .addClass("rsd_upload_button")
915924 .click(function(){
916925 _this.current_provider = 'upload';
917 - _this.showUploadTab( );
 926+ _this.UpdateUploadResults( );
918927 });
919928 $searchForm.append( $uploadButton );
920929 /*
@@ -932,11 +941,11 @@
933942 /**
934943 * Shows the upload tab loader and issues a call to showUploadForm
935944 */
936 - showUploadTab: function() {
937 - mw.log( "showUploadTab::" );
 945+ UpdateUploadResults: function() {
 946+ mw.log( "UpdateUploadResults::" );
938947 var _this = this;
939948 // set it to loading:
940 - $j( '#rsd_results_container' ).loadingSpinner();
 949+ this.$resultsContainer.loadingSpinner();
941950 //Show the upload form
942951 mw.load( ['$j.fn.simpleUploadForm'], function() {
943952 var provider = _this.content_providers['this_wiki'];
@@ -982,7 +991,7 @@
983992 } )
984993 .loadingSpinner()
985994 )
986 - $j( '#rsd_results_container' ).html(
 995+ this.$resultsContainer.html(
987996 $j('<table />').append(
988997 $uploadTableRow
989998 )
@@ -1020,28 +1029,27 @@
10211030 },
10221031
10231032 /**
1024 - * Show the current tab ( based on current_provider var )
 1033+ * Refresh the results container ( based on current_provider var )
10251034 */
1026 - showCurrentTab: function() {
 1035+ UpdateResults: function() {
 1036+ this.$filtersContainer.hide();
10271037 if ( this.current_provider == 'upload' ) {
1028 - this.showUploadTab();
 1038+ this.UpdateUploadResults();
10291039 } else {
1030 - this.showSearchTab( this.current_provider, false );
 1040+ this.UpdateSearchResults( this.current_provider, false );
10311041 }
10321042 },
10331043
10341044 /**
1035 - * Show the search tab for a given providerName
 1045+ * Show updated search results for a given providerName
10361046 *
10371047 * @param {String} providerName name of the content provider
10381048 * @param {Bollean} resetPaging if the pagging should be reset
10391049 */
1040 - showSearchTab: function( providerName, resetPaging ) {
1041 - mw.log( "f:showSearchTab::" + providerName );
 1050+ UpdateSearchResults: function( providerName, resetPaging ) {
 1051+ mw.log( "f:UpdateSearchResults::" + providerName );
10421052
10431053 var draw_direct_flag = true;
1044 -
1045 - // Else do showSearchTab
10461054 var provider = this.content_providers[ providerName ];
10471055
10481056 // Check if we need to update:
@@ -1066,16 +1074,16 @@
10671075 }
10681076
10691077 if ( $j ( '#rsd_q' ).val().length == 0 ) {
1070 - $j( '#rsd_results_container' ).empty()
1071 - $j( '#rsd_results_container' ).text( 'Please insert a search string above.' );
 1078+ this.$resultsContainer.empty();
 1079+ this.$resultsContainer.text( 'Please insert a search string above.' );
10721080 return;
10731081 }
10741082
10751083 // Set the content to loading while we do the search:
1076 - $j( '#rsd_results_container' ).html( mw.loading_spinner() );
1077 -
 1084+ this.$resultsContainer.html( mw.loading_spinner() );
 1085+
10781086 // Make sure the search library is loaded and issue the search request
1079 - this.getProviderResults( provider );
 1087+ this.PerformProviderSearch( provider );
10801088 },
10811089
10821090 /*
@@ -1171,22 +1179,74 @@
11721180 } );
11731181 },
11741182
 1183+
 1184+ createSearchFilters: function( current_provider ) {
 1185+ var _this = this;
 1186+ mw.log( 'f: CreateSearchFilters ' );
 1187+
 1188+ this.CreateFilterBox( 'rsd_media_type_filter',
 1189+ 'Media',
 1190+ { Video: 'video',
 1191+ Image: 'image',
 1192+ Audio: 'audio' } );
 1193+
 1194+ this.CreateFilterBox( 'rsd_provider_filter',
 1195+ 'Provider',
 1196+ current_provider.aggregated_providers );
 1197+
 1198+ },
 1199+
11751200 /**
1176 - * Get the search results for a given content provider
 1201+ * Creates a single filter box with given options and appends it to the filter container.
 1202+ *
 1203+ * @id {string} unique id for this filter box an residing elements
 1204+ * @title {string} title of the filter box
 1205+ * @options {array} array of strings describing the options in the filter box
 1206+ *
 1207+ */
 1208+
 1209+ CreateFilterBox: function( id, title, options ) {
 1210+ $box = $j( '<div />' ).addClass( 'ui-filter-box' ).attr({
 1211+ 'id': id
 1212+ });
 1213+
 1214+ $title = $j( '<div />' ).addClass( 'ui-filter-title' ).text( title );
 1215+ $box.append( $title );
 1216+
 1217+ for (option in options) {
 1218+ $option = $j( '<div />' ).addClass( 'ui-filter-option' ).text( option );
 1219+
 1220+ $checkbox = $j( '<input />' )
 1221+ .attr( {
 1222+ type: 'checkbox',
 1223+ name: id + '_' + title + '_' + options.option,
 1224+ value: options.option,
 1225+ checked: true
 1226+ } );
 1227+
 1228+ $option.prepend( $checkbox );
 1229+ $box.append( $option );
 1230+ }
 1231+
 1232+ this.$filtersContainer.append($box);
 1233+ },
 1234+
 1235+ /**
 1236+ * Performs the search for a given content provider
11771237 *
1178 - * Sets up binding to showResults once search providers results are ready
 1238+ * Calls showResults once search results are ready
11791239 *
11801240 * @param {Object} provider the provider to be searched.
11811241 */
1182 - getProviderResults: function( provider ) {
 1242+ PerformProviderSearch: function( provider ) {
11831243 var _this = this;
1184 - mw.log('f: getProviderResults ' );
 1244+ mw.log( 'f: PerformProviderSearch ' );
11851245 // First check if we should even run the search at all (can we import / insert
11861246 // into the page? )
11871247 if ( !this.isProviderLocal( provider ) && this.import_url_mode == 'autodetect' ) {
11881248 // provider is not local check if we can support the import mode:
11891249 this.checkForCopyURLSupport( function() {
1190 - _this.getProviderResults( provider );
 1250+ _this.PerformProviderSearch( provider );
11911251 } );
11921252 return false;
11931253 } else if ( !this.isProviderLocal( provider ) && this.import_url_mode == 'none' ) {
@@ -1194,7 +1254,7 @@
11951255 // combined results are harder to error handle just ignore that repo
11961256 provider.sObj.loading = false;
11971257 } else {
1198 - $j( '#rsd_results_container' ).html(
 1258+ this.$resultsContainer.html(
11991259 gM( 'mwe-no_import_by_url' )
12001260 )
12011261 }
@@ -1287,24 +1347,26 @@
12881348 mw.log( 'f:showResults::' + this.current_provider );
12891349 var _this = this;
12901350 var o = '';
1291 - var tabSelector = '#rsd_results_container';
12921351 var provider = this.content_providers[ this.current_provider ];
12931352
12941353
12951354 // The "upload" tab has special results output target rather than top level
12961355 // resutls container.
1297 - // @@todo Clean up by moving the rsd_results_container down in the DOM for selected tab display.
1298 - // ( will be need once we add left side filter controls
1299 - // that are exclusive to some providers )
13001356 if ( this.current_provider == 'upload' ) {
13011357 $resultsContainer = $j('#upload_bin');
13021358 var provider = _this.content_providers['this_wiki'];
13031359 } else {
13041360 var provider = this.content_providers[ this.current_provider ];
1305 - $resultsContainer = $j('#rsd_results_container');
 1361+ $resultsContainer = this.$resultsContainer;
13061362
13071363 // Add the results header:
13081364 $resultsContainer.empty().append( this.createResultsHeader() )
 1365+
 1366+ // Enable search filters, if the provider supports them.
 1367+ if ( provider.show_filters ) {
 1368+ // this.$filtersContainer.show();
 1369+ this.createSearchFilters( provider );
 1370+ }
13091371 }
13101372
13111373 // Empty the existing results:
@@ -1479,7 +1541,7 @@
14801542 $j( _this.target_container ).find( '#rsd_resource_edit' ).remove();
14811543
14821544 // Hide the results container
1483 - $j( '#rsd_results_container' ).hide();
 1545+ this.$resultsContainer.hide();
14841546
14851547 var pt = $j( _this.target_container ).html();
14861548
@@ -1705,7 +1767,7 @@
17061768 $j( '#rsd_preview_display' ).remove();
17071769
17081770 // Restore the resource container:
1709 - $j( '#rsd_results_container' ).show();
 1771+ this.$resultsContainer.show();
17101772
17111773 // Restore the title:
17121774 $j( _this.target_container ).dialog( 'option', 'title', gM( 'mwe-add_media_wizard' ) );
@@ -2766,7 +2828,7 @@
27672829 provider.offset -= provider.limit;
27682830 if ( provider.offset < 0 )
27692831 provider.offset = 0;
2770 - _this.showCurrentTab();
 2832+ _this.UpdateResults();
27712833 } );
27722834 $pagingControl.prepend( $prevLink );
27732835 }
@@ -2782,7 +2844,7 @@
27832845 .text( nextLinkText )
27842846 .click( function() {
27852847 provider.offset += provider.limit;
2786 - _this.showCurrentTab();
 2848+ _this.UpdateResults();
27872849 } );
27882850 $pagingControl.append( $nextLink );
27892851 }
@@ -2798,10 +2860,10 @@
27992861 mw.log( 'select tab: ' + provider_id );
28002862 this.current_provider = provider_id;
28012863 if ( this.current_provider == 'upload' ) {
2802 - this.showUploadTab();
 2864+ this.UpdateUploadResults();
28032865 } else {
28042866 // update the search results:
2805 - this.showCurrentTab();
 2867+ this.UpdateResults();
28062868 }
28072869 },
28082870

Comments

#Comment by Mdale (talk | contribs)   07:11, 25 January 2010
  • I don't think the upper case method names is very consistent with everything else.
  • Any reason 'aggregated_providers' is listed "title text" first?
  • Any reason the 'aggregated_providers' does not use existing localized titles ie: 'rsd-wiki_commons-title' : "Wikimedia Commons"
  • There should be some logic of "filters" to api mapping handled in kalturaSearch.js. Since what filters a given provider supports is specific to the provider. Not specific to the parent RemoteSearchDriver. Or maybe add a separate "filters" class that can be passed configuration from a given provider that the RemoteSearchDriver calls to build out out the set of filters in the interface.
  • Also it appears in

  UpdateResults: function() {
       this.$filtersContainer.hide();

you hide the $filtersContainer, for experimental feature you should make it easy to turn it on or off via configuration not have to modify the code ie your 'show_filters': true, configuration option could have been the config switch to test the filters.

  • When filters are unhidden, other providers that don't support filters show the filters box.
  • I think the "filters" left hand side should be collapsible / expandable with a menu item in the rsd_results_header. ( with the menu item not appearing if the provider does not support the "filters" )


#Comment by 😂 (talk | contribs)   12:54, 10 August 2010

Ping. Was there ever followup to this, or are any/all these issues still present?

#Comment by Spamsplace (talk | contribs)   17:45, 10 August 2010

Most of these issues have been resolved in a later commit: [1]

At the time I wasn't aware of the method to properly mark follow-ups. To this day I do not know how to revise the commit message.

As for issues #6, #7: I believe these are still open issues, awaiting a web designer.

#Comment by Mdale (talk | contribs)   22:38, 10 August 2010

I am going to mark this as resolved since the remaining issues would be part of a design effort not part of a fix me for this commit.

Status & tagging log