jenkins-bot has submitted this change and it was merged. Change subject: Load nearby by url coordinates and load by page title ......................................................................
Load nearby by url coordinates and load by page title Some test urls: http://localhost:8080/w/index.php?title=Special:Nearby&debug=true#/coord/37.785185,-122.3989839 http://localhost:8080/w/index.php?title=Special:Nearby&debug=true#/page/W_San_Francisco Bug: 49413 Change-Id: I29bf5b25f8a02659a81cbd10227a9175b1517f32 --- M javascripts/modules/nearby/Nearby.js M javascripts/modules/nearby/NearbyApi.js M javascripts/specials/nearby.js M tests/qunit/modules/nearby/test_Nearby.js M tests/qunit/modules/nearby/test_NearbyApi.js 5 files changed, 162 insertions(+), 44 deletions(-) Approvals: Jdlrobson: Looks good to me, approved jenkins-bot: Verified diff --git a/javascripts/modules/nearby/Nearby.js b/javascripts/modules/nearby/Nearby.js index 8b0c399..948057a 100644 --- a/javascripts/modules/nearby/Nearby.js +++ b/javascripts/modules/nearby/Nearby.js @@ -88,7 +88,7 @@ options.errorType = errorType; _super.call( self, options ); } ); - } else if ( options.latitude && options.longitude ) { + } else if ( ( options.latitude && options.longitude ) || options.pageTitle ) { // Flush any existing list of pages options.pages = []; @@ -103,21 +103,37 @@ _super.apply( this, arguments ); }, _find: function ( options ) { - var result = $.Deferred(), self = this; + var result = $.Deferred(), self = this, + pagesSuccess, pagesError; + + // Handler for successful query + pagesSuccess = function ( pages ) { + options.pages = pages; + if ( pages && pages.length === 0 ) { + options.error = self.errorMessages.empty; + } + self._isLoading = false; + result.resolve( options ); + }; + + // Handler for failed queries + pagesError = function () { + self._isLoading = false; + options.error = self.errorMessages.server; + result.resolve( options ); + }; + if ( options.latitude && options.longitude ) { - this.nearbyApi.getPages( { latitude: options.latitude, longitude: options.longitude }, - this.range, options.exclude ).done( function ( pages ) { - options.pages = pages; - if ( pages && pages.length === 0 ) { - options.error = self.errorMessages.empty; - } - self._isLoading = false; - result.resolve( options ); - } ).fail( function () { - self._isLoading = false; - options.error = self.errorMessages.server; - result.resolve( options ); - } ); + this.nearbyApi.getPages( + { latitude: options.latitude, longitude: options.longitude }, + this.range, options.exclude + ) + .done( pagesSuccess ) + .fail( pagesError ); + } else if ( options.pageTitle ) { + this.nearbyApi.getPagesAroundPage( options.pageTitle, this.range) + .done( pagesSuccess ) + .fail( pagesError ); } else { if ( options.errorType ) { options.error = this.errorMessages[ options.errorType ]; @@ -134,10 +150,9 @@ this._postRenderLinks(); }, _postRenderLinks: function () { - this.$( 'a' ).on( 'click', function ( ev ) { + this.$( 'a' ).on( 'click', function () { // name funnel for watchlists to catch subsequent uploads $.cookie( 'mwUploadsFunnel', 'nearby', { expires: new Date( new Date().getTime() + 60000) } ); - window.location.hash = '#' + $( ev.currentTarget ).attr( 'name' ); } ); } } ); diff --git a/javascripts/modules/nearby/NearbyApi.js b/javascripts/modules/nearby/NearbyApi.js index 18aa9d3..231d5fc 100644 --- a/javascripts/modules/nearby/NearbyApi.js +++ b/javascripts/modules/nearby/NearbyApi.js @@ -64,28 +64,55 @@ return mw.msg( msg, mw.language.convertNumber( d ) ); }, /** - * Renders an error in the existing view + * Returns a list of pages around a given point * - * @param {Object} location: In form { latitude: 0, longitude: 2 } + * @param {Object} coords: In form { latitude: 0, longitude: 2 } * @param {Number} range: Number of meters to perform a geosearch for * @param {String} exclude: Name of a title to exclude from the list of results * @return {jQuery.Deferred} Object taking list of pages as argument */ - getPages: function ( location, range, exclude ) { - var d = $.Deferred(), self = this; - this.get( { + getPages: function ( coords, range, exclude ) { + return this._search( { + ggscoord: [ coords.latitude, coords.longitude ] + }, range, exclude ); + }, + + /** + * Gets the pages around a page. It excludes itself from the search + * + * @param {String} page: Page title like "W_San_Francisco" + * @param {Number} range: Number of meters to perform a geosearch for + * @return {jQuery.Deferred} Object taking list of pages as argument + */ + getPagesAroundPage: function ( page, range ) { + return this._search( { ggspage: page }, range, page ); + }, + + /** + * Searches for pages nearby + * + * @param {Object} params: Parameters to use for searching + * @param {Number} range: Number of meters to perform a geosearch for + * @param {String} exclude: Name of a title to exclude from the list of results + * @return {jQuery.Deferred} Object taking list of pages as argument + */ + _search: function ( params, range, exclude ) { + var d = $.Deferred(), self = this, loc, requestParams; + + requestParams = { action: 'query', colimit: 'max', prop: 'pageimages|coordinates', pithumbsize: mw.config.get( 'wgMFThumbnailSizes' ).small, pilimit: limit, generator: 'geosearch', - ggscoord: [ location.latitude, location.longitude ], ggsradius: range, ggsnamespace: ns, ggslimit: limit - }, - { + }; + $.extend( requestParams, params); + + this.get( requestParams, { dataType: endpoint ? 'jsonp' : 'json', url: endpoint || this.apiUrl } ).then( function ( resp ) { @@ -98,10 +125,31 @@ } else { pages = {}; } + // FIXME: API returns object when array would make much sense pages = $.map( pages, function ( i ) { return i; } ); + + // If we have coordinates then set them so that the results are sorted by + // distance + if ( params.ggscoord ) { + loc = { latitude: params.ggscoord[0], longitude: params.ggscoord[1] }; + } + // If we have no coords (searching for a page's nearby), find the + // page in the results and get its coords. + if ( params.ggspage ) { + $.each( pages, function ( i, page ) { + if ( params.ggspage === page.title ) { + loc = { + latitude: page.coordinates[0].lat, + longitude: page.coordinates[0].lon + }; + } + } ); + } + + // Process the pages pages = $.map( pages, function ( page, i ) { var coords, lngLat, thumb; @@ -114,10 +162,10 @@ } page.anchor = 'item_' + i; page.url = mw.util.getUrl( page.title ); - if ( page.coordinates ) { // FIXME: protect against bug 47133 (remove when resolved) + if ( page.coordinates && loc ) { // FIXME: protect against bug 47133 (remove when resolved) coords = page.coordinates[0]; lngLat = { latitude: coords.lat, longitude: coords.lon }; - page.dist = calculateDistance( location, lngLat ); + page.dist = calculateDistance( loc, lngLat ); page.latitude = coords.lat; page.longitude = coords.lon; page.proximity = self._distanceMessage( page.dist ); @@ -136,8 +184,10 @@ } ); d.resolve( pages ); } ); + return d; } + } ); M.define( 'modules/nearby/NearbyApi', NearbyApi ); diff --git a/javascripts/specials/nearby.js b/javascripts/specials/nearby.js index 5d6c915..fc2e270 100644 --- a/javascripts/specials/nearby.js +++ b/javascripts/specials/nearby.js @@ -1,27 +1,20 @@ ( function ( M, $ ) { - var icon, - Icon = M.require( 'Icon' ), + var Icon = M.require( 'Icon' ), Nearby = M.require( 'modules/nearby/Nearby' ); $( function () { var nearby, - options = { el: $( '#mw-mf-nearby' ), useCurrentLocation: true }, - $btn = $( '#secondary-button' ); + options = { el: $( '#mw-mf-nearby' ) }, + $btn = $( '#secondary-button' ), + icon, $icon; - function refresh() { - if ( nearby ) { - nearby.initialize( options ); - } else { - nearby = new Nearby( options ); - } - } - - // replace user button with refresh button + // Remove user button if ( $btn.length ) { $btn.remove(); } + // Create refresh button on the header icon = new Icon( { name: 'refresh', id: 'secondary-button', additionalClassNames: 'main-header-button', @@ -29,10 +22,48 @@ title: mw.msg( 'mobile-frontend-nearby-refresh' ), label: mw.msg( 'mobile-frontend-nearby-refresh' ) } ); - $( icon.toHtmlString() ).on( 'click', refresh ). - appendTo( '.header' ); + $icon = $( icon.toHtmlString() ).on( 'click', refreshCurrentLocation ).appendTo( '.header' ); - refresh(); + function refresh( options ) { + if ( nearby ) { + nearby.initialize( options ); + } else { + nearby = new Nearby( options ); + } + } + + // Routing on the nearby view + + /* + * #/coords/lat,long + */ + M.router.route( /^\/coord\/(-?\d+(?:\.\d+)?),(-?\d+(?:\.\d+)?)/, function ( lat, lon ) { + $icon.hide(); + // Search with coordinates + refresh( $.extend( {}, options, { + latitude: lat, + longitude: lon + } ) ); + } ); + /* + * #/page/PageTitle + */ + M.router.route( /^\/page\/(.+)$/, function ( pageTitle ) { + $icon.hide(); + refresh( $.extend( {}, options, { pageTitle: pageTitle } ) ); + } ); + function refreshCurrentLocation() { + $icon.show(); + refresh( $.extend( {}, options, { useCurrentLocation: true } ) ); + } + /* + * Anything else search with current location + * FIXME: The regex has to negate the rest of the routes because every time we + * define a route with router.route that route gets matched against the + * current hash. + */ + M.router.route( /^(?!.coord|.page).*$/, refreshCurrentLocation ); + } ); }( mw.mobileFrontend, jQuery ) ); diff --git a/tests/qunit/modules/nearby/test_Nearby.js b/tests/qunit/modules/nearby/test_Nearby.js index 5a7b18f..5bcd38d 100644 --- a/tests/qunit/modules/nearby/test_Nearby.js +++ b/tests/qunit/modules/nearby/test_Nearby.js @@ -8,7 +8,7 @@ setup: function() { this.spy = this.sandbox.stub( NearbyApi.prototype, 'getPages' ). returns( $.Deferred().resolve( [ - ] ) ); + ] ) ); } } ); @@ -78,4 +78,17 @@ n.errorMessages.server.heading, 'Check it is the correct heading' ); } ); + QUnit.module( 'MobileFrontend modules/nearby/Nearby (4 - Around page)', { + setup: function() { + this.spy = this.sandbox.stub( NearbyApi.prototype, 'getPagesAroundPage' ). + returns( $.Deferred().reject() ); + } + } ); + + QUnit.test( '#getting a title will trigger a different API method', 1, function( assert ) { + var $el = $( '<div>' ), pageTitle = 'Hello Friends!'; + new Nearby( { pageTitle: pageTitle, range: 1000, el: $el } ); + assert.ok( this.spy.calledWithMatch( pageTitle, 1000 ), 'Check API got called' ); + } ); + }( mw.mobileFrontend, jQuery ) ); diff --git a/tests/qunit/modules/nearby/test_NearbyApi.js b/tests/qunit/modules/nearby/test_NearbyApi.js index 29178ea..1557224 100644 --- a/tests/qunit/modules/nearby/test_NearbyApi.js +++ b/tests/qunit/modules/nearby/test_NearbyApi.js @@ -51,4 +51,13 @@ } ); } ); +QUnit.test( '#getPagesAroundPage', 4, function( assert ) { + m.getPagesAroundPage( 'The Montgomery (San Francisco)' ).done( function( pages ) { + assert.strictEqual( pages.length, 2 ); + assert.strictEqual( pages[1].title, 'W San Francisco' ); + assert.strictEqual( pages[1].pageimageClass, 'list-thumb-none list-thumb-x' ); + assert.strictEqual( pages[1].dist.toPrecision( 6 ), "22.2639" ); + } ); +} ); + }( mw.mobileFrontend, jQuery ) ); -- To view, visit https://gerrit.wikimedia.org/r/168208 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I29bf5b25f8a02659a81cbd10227a9175b1517f32 Gerrit-PatchSet: 16 Gerrit-Project: mediawiki/extensions/MobileFrontend Gerrit-Branch: master Gerrit-Owner: Jhernandez <[email protected]> Gerrit-Reviewer: Awjrichards <[email protected]> Gerrit-Reviewer: Jdlrobson <[email protected]> Gerrit-Reviewer: Jhernandez <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
