Jdlrobson has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/349252 )
Change subject: Please don't abuse global events on M object when you dont need
to
......................................................................
Please don't abuse global events on M object when you dont need to
The SearchOverlay makes use of the global event emitter, apparently
just so it can log events. This is completely unnecessary as the
search logger is created at the same time as the search overlay.
Instead of doing this use the fact that a SearchOverlay has an
EventEmitter mixin and use those events instead.
If we want to use global events we should probably be using
mw.track instead as it is supported by core.
Change-Id: I862114e59e1b7816bfc5b1bfd31e703bda463d91
---
M resources/mobile.search/MobileWebSearchLogger.js
M resources/mobile.search/SearchOverlay.js
M resources/skins.minerva.scripts/search.js
3 files changed, 19 insertions(+), 16 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/52/349252/1
diff --git a/resources/mobile.search/MobileWebSearchLogger.js
b/resources/mobile.search/MobileWebSearchLogger.js
index 38d364e..ea8e533 100644
--- a/resources/mobile.search/MobileWebSearchLogger.js
+++ b/resources/mobile.search/MobileWebSearchLogger.js
@@ -99,14 +99,15 @@
* Convenience function that wires up an instance of the
* MobileWebSearchLogger class to the search-* events emitted by the
* search overlay.
+ * @param {SearchOverlay} searchOverlay
*/
- MobileWebSearchLogger.register = function () {
+ MobileWebSearchLogger.register = function ( searchOverlay ) {
var logger = new MobileWebSearchLogger();
- M.on( 'search-show', logger.onSearchShow.bind( logger ) );
- M.on( 'search-start', logger.onSearchStart.bind( logger ) );
- M.on( 'search-results', logger.onSearchResults.bind( logger ) );
- M.on( 'search-result-click', logger.onSearchResultClick.bind(
logger ) );
+ searchOverlay.on( 'search-show', logger.onSearchShow.bind(
logger ) );
+ searchOverlay.on( 'search-start', logger.onSearchStart.bind(
logger ) );
+ searchOverlay.on( 'search-results',
logger.onSearchResults.bind( logger ) );
+ searchOverlay.on( 'search-result-click',
logger.onSearchResultClick.bind( logger ) );
};
M.define( 'mobile.search/MobileWebSearchLogger', MobileWebSearchLogger
);
diff --git a/resources/mobile.search/SearchOverlay.js
b/resources/mobile.search/SearchOverlay.js
index 5830e93..120f160 100644
--- a/resources/mobile.search/SearchOverlay.js
+++ b/resources/mobile.search/SearchOverlay.js
@@ -225,7 +225,7 @@
* result in the set of results
* @property {jQuery.Event} originalEvent The original
event
*/
- M.emit( 'search-result-click', {
+ this.emit( 'search-result-click', {
result: $result,
resultIndex: this.$results.index( $result ),
originalEvent: ev
@@ -262,7 +262,7 @@
// Show a spinner on top of search results
this.$spinner = this.$( '.spinner-container' );
- M.on( 'search-start', function ( searchData ) {
+ this.on( 'search-start', function ( searchData ) {
if ( timer ) {
clearSearch();
}
@@ -270,7 +270,7 @@
self.$spinner.show();
}, SEARCH_SPINNER_DELAY );
} );
- M.on( 'search-results', clearSearch );
+ this.on( 'search-results', clearSearch );
// Hide the clear button if the search input is empty
if ( self.$input.val() === '' ) {
@@ -291,7 +291,7 @@
/**
* @event search-show Fired after the search overlay is
shown
*/
- M.emit( 'search-show' );
+ this.emit( 'search-show' );
},
/**
@@ -340,7 +340,7 @@
* sent
* @property {Object} data
related to the current search
*/
- M.emit( 'search-start', {
+ self.emit( 'search-start', {
query: query,
delay: delay
} );
@@ -373,7 +373,7 @@
* @property
{Object[]} results The results returned by the search
* API
*/
- M.emit(
'search-results', {
+ self.emit(
'search-results', {
results: data.results
} );
}
diff --git a/resources/skins.minerva.scripts/search.js
b/resources/skins.minerva.scripts/search.js
index 9f0ebdc..10d73ee 100644
--- a/resources/skins.minerva.scripts/search.js
+++ b/resources/skins.minerva.scripts/search.js
@@ -2,6 +2,7 @@
var SearchOverlay = M.require( 'mobile.search/SearchOverlay' ),
SearchGateway = M.require( 'mobile.search.api/SearchGateway' ),
router = require( 'mediawiki.router' ),
+ searchLogger = M.require( 'mobile.search/MobileWebSearchLogger'
),
browser = M.require( 'mobile.startup/Browser' ).getSingleton();
/**
@@ -11,7 +12,8 @@
* @ignore
*/
function openSearchOverlay( ev ) {
- var $this = $( this ),
+ var overlay,
+ $this = $( this ),
searchTerm = $this.val(),
placeholder = $this.attr( 'placeholder' );
@@ -19,13 +21,15 @@
// The loading of SearchOverlay should never be done inside a
callback
// as this will result in issues with input focus
// see https://phabricator.wikimedia.org/T156508#2977463
- new SearchOverlay( {
+ overlay = new SearchOverlay( {
router: router,
gatewayClass: SearchGateway,
api: new mw.Api(),
searchTerm: searchTerm,
placeholderMsg: placeholder
- } ).show();
+ } );
+ searchLogger.register( overlay );
+ overlay.show();
router.navigate( '/search' );
}
@@ -48,7 +52,5 @@
// Apparently needed for main menu to work correctly.
.prop( 'readonly', true );
}
-
- M.require( 'mobile.search/MobileWebSearchLogger' ).register();
}( mw.mobileFrontend, jQuery ) );
--
To view, visit https://gerrit.wikimedia.org/r/349252
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I862114e59e1b7816bfc5b1bfd31e703bda463d91
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: master
Gerrit-Owner: Jdlrobson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
