jenkins-bot has submitted this change and it was merged.
Change subject: Make API request after 500 ms
......................................................................
Make API request after 500 ms
If the user has abandoned the link or dwelled on a different link, then
don't make the API request.
Changes:
* Fix a bug in the linkDwell reducer where the activeLink was always
being set to undefined.
* Add the private fetch action creator.
* Make the linkDwell action dispatch the result of the fetch action
creator after 500ms.
Supporting changes:
* Make the link* action creators take DOM elements rather than jQuery
instances so that:
1. Testing whether the state tree has changed is an identity
comparison.
2. jQuery instances are constructed only when required.
* Document the ext.popups.Gateway type.
* Document the Redux.Store type.
* Rename the "previews-page-title" data attribute to
"page-previews-title" to avoid confusion.
Change-Id: I0b1cf3337a6f8d6450ad2bd127cb292ebb73af4f
---
M resources/ext.popups/actions.js
M resources/ext.popups/boot.js
M resources/ext.popups/changeListener.js
M resources/ext.popups/gateway.js
M resources/ext.popups/processLinks.js
M resources/ext.popups/reducers.js
M tests/qunit/ext.popups/actions.test.js
M tests/qunit/ext.popups/processLinks.test.js
8 files changed, 176 insertions(+), 32 deletions(-)
Approvals:
jenkins-bot: Verified
Phuedx: Looks good to me, approved
diff --git a/resources/ext.popups/actions.js b/resources/ext.popups/actions.js
index 4aea7a2..80a3587 100644
--- a/resources/ext.popups/actions.js
+++ b/resources/ext.popups/actions.js
@@ -1,4 +1,4 @@
-( function ( mw ) {
+( function ( mw, $ ) {
var actions = {},
types = {
@@ -15,7 +15,8 @@
COG_CLICK: 'COG_CLICK',
SETTINGS_DIALOG_RENDERED: 'SETTINGS_DIALOG_RENDERED',
SETTINGS_DIALOG_CLOSED: 'SETTINGS_DIALOG_CLOSED'
- };
+ },
+ FETCH_START_DELAY = 500; // ms.
/**
* Represents Link Previews booting.
@@ -44,16 +45,54 @@
};
/**
+ * Represents Page Previews fetching data via the
[gateway](./gateway.js).
+ *
+ * @param {ext.popups.Gateway} gateway
+ * @param {String} title
+ * @return {Redux.Thunk}
+ */
+ function fetch( gateway, title ) {
+ return function ( dispatch ) {
+ dispatch( {
+ type: types.FETCH_START,
+ title: title
+ } );
+
+ gateway( title )
+ .fail( function () {
+ dispatch( {
+ type: types.FETCH_FAILED
+ } );
+ } )
+ .done( function ( result ) {
+ dispatch( {
+ type: types.FETCH_END,
+ result: result
+ } );
+ } );
+ };
+ }
+
+ /**
* Represents the user dwelling on a link, either by hovering over it
with
* their mouse or by focussing it using their keyboard or an assistive
device.
*
- * @param {jQuery} $el
- * @return {Object}
+ * @param {Element} el
+ * @param {Function} gateway
+ * @return {Redux.Thunk}
*/
- actions.linkDwell = function ( $el ) {
- return {
- type: types.LINK_DWELL,
- el: $el
+ actions.linkDwell = function ( el, gateway ) {
+ return function ( dispatch, getState ) {
+ dispatch( {
+ type: types.LINK_DWELL,
+ el: el
+ } );
+
+ setTimeout( function () {
+ if ( getState().preview.activeLink === el ) {
+ dispatch( fetch( gateway, $( el ).data(
'page-previews-title' ) ) );
+ }
+ }, FETCH_START_DELAY );
};
};
@@ -62,13 +101,13 @@
* from it or by shifting focus to another UI element using their
keyboard or
* an assistive device.
*
- * @param {jQuery} $el
+ * @param {Element} el
* @return {Object}
*/
- actions.linkAbandon = function ( $el ) {
+ actions.linkAbandon = function ( el ) {
return {
type: types.LINK_ABANDON,
- el: $el
+ el: el
};
};
@@ -76,13 +115,13 @@
* Represents the user clicking on a link with their mouse, keyboard,
or an
* assistive device.
*
- * @param {jQuery} $el
+ * @param {Element} el
* @return {Object}
*/
- actions.linkClick = function ( $el ) {
+ actions.linkClick = function ( el ) {
return {
type: 'LINK_CLICK',
- el: $el
+ el: el
};
};
@@ -101,4 +140,4 @@
mw.popups.actions = actions;
mw.popups.actionTypes = types;
-}( mediaWiki ) );
+}( mediaWiki, jQuery ) );
diff --git a/resources/ext.popups/boot.js b/resources/ext.popups/boot.js
index 4a184ef..db54fdc 100644
--- a/resources/ext.popups/boot.js
+++ b/resources/ext.popups/boot.js
@@ -10,9 +10,11 @@
];
/**
- * Return whether the user is in the experiment group
+ * Creates an experiment with sensible values for the depenencies.
*
- * @return {Boolean}
+ * See `mw.popups.createExperiment`.
+ *
+ * @return {Function}
*/
function isUserInCondition() {
var userSettings = mw.popups.createUserSettings( mw.storage,
mw.user );
@@ -25,13 +27,24 @@
}
/**
+ * Creates a gateway with sensible values for the dependencies.
+ *
+ * See `mw.popups.createGateway`.
+ *
+ * @return {ext.popups.Gateway}
+ */
+ function createGateway() {
+ return mw.popups.createGateway( new mw.Api() );
+ }
+
+ /**
* Subscribes the registered change listeners to the
* [store](http://redux.js.org/docs/api/Store.html#store).
*
* Change listeners are registered by setting a property on
* `mw.popups.changeListeners`.
*
- * @param {Store} store
+ * @param {Redux.Store} store
*/
function registerChangeListeners( store, actions ) {
$.each( mw.popups.changeListeners, function ( _,
changeListenerFactory ) {
@@ -45,7 +58,7 @@
* Binds the actions (or "action creators") to the
* [store](http://redux.js.org/docs/api/Store.html#store).
*
- * @param {Store} store
+ * @param {Redux.Store} store
* @return {Object}
*/
function createBoundActions( store ) {
@@ -67,7 +80,8 @@
var compose = Redux.compose,
store,
actions,
- generateToken = mw.user.generateRandomSessionId;
+ generateToken = mw.user.generateRandomSessionId,
+ gateway = createGateway();
// If debug mode is enabled, then enable Redux DevTools.
if ( mw.config.get( 'debug' ) === true ) {
@@ -98,7 +112,7 @@
previewLinks
.on( 'mouseover focus', function () {
- actions.linkDwell( this );
+ actions.linkDwell( this, gateway );
} )
.on( 'mouseout blur', function () {
actions.linkAbandon( this );
diff --git a/resources/ext.popups/changeListener.js
b/resources/ext.popups/changeListener.js
index 75ee53c..4077b3b 100644
--- a/resources/ext.popups/changeListener.js
+++ b/resources/ext.popups/changeListener.js
@@ -18,7 +18,7 @@
* See
[Store#subscribe](http://redux.js.org/docs/api/Store.html#subscribe)
* for more information about what change listeners may and may not do.
*
- * @param {Store} store
+ * @param {Redux.Store} store
* @param {ext.popups.ChangeListener} callback
*/
mw.popups.registerChangeListener = function ( store, callback ) {
diff --git a/resources/ext.popups/gateway.js b/resources/ext.popups/gateway.js
index 7db25f1..c58cd84 100644
--- a/resources/ext.popups/gateway.js
+++ b/resources/ext.popups/gateway.js
@@ -7,6 +7,11 @@
CACHE_LIFETIME = 300; // Public and private cache lifetime (5
minutes)
/**
+ * @typedef {Function} ext.popups.Gateway
+ * @param {String} title
+ */
+
+ /**
* Creates a function that fetches all of the data required to give the
user a
* preview of the page from the MediaWiki API given the title of the
page (see
* `mw.popups.processLinks` for the definition of "title").
@@ -15,7 +20,7 @@
* will reject; otherwise, it'll resolve.
*
* @param {mw.Api} api
- * @return {Function}
+ * @return {ext.popups.Gateway}
*/
mw.popups.createGateway = function ( api ) {
return function ( title ) {
@@ -46,7 +51,6 @@
}
} )
.then( function ( data ) {
-
// If the response is empty, i.e.
data.query.pages is empty or isn't
// set, then reject rather than resolve.
if (
diff --git a/resources/ext.popups/processLinks.js
b/resources/ext.popups/processLinks.js
index 7d992bc..0f46464 100644
--- a/resources/ext.popups/processLinks.js
+++ b/resources/ext.popups/processLinks.js
@@ -83,7 +83,7 @@
// Is titleText in a content namespace?
title = mw.Title.newFromText( titleText );
if ( title && ( $.inArray( title.namespace,
contentNamespaces ) >= 0 ) ) {
- $( this ).data( 'previews-page-title',
titleText );
+ $( this ).data( 'page-previews-title',
titleText );
return true;
}
diff --git a/resources/ext.popups/reducers.js b/resources/ext.popups/reducers.js
index 568cbf6..3a840b2 100644
--- a/resources/ext.popups/reducers.js
+++ b/resources/ext.popups/reducers.js
@@ -39,14 +39,12 @@
} );
case mw.popups.actionTypes.LINK_DWELL:
return $.extend( OO.copy( state ), {
- activeLink: action.activeLink,
+ activeLink: action.el,
interactionStarted:
action.interactionStarted,
isDelayingFetch: true,
linkInteractionToken:
action.linkInteractionToken
} );
case mw.popups.actionTypes.LINK_ABANDON:
- // Should the action handle dispatching a
FETCH_FAILED
- // if a new fetch has begun? Or should the
reducer?
return $.extend( OO.copy( state ), {
previousActiveLink: state.activeLink,
activeLink: undefined,
diff --git a/tests/qunit/ext.popups/actions.test.js
b/tests/qunit/ext.popups/actions.test.js
index 0110bf5..e46b92d 100644
--- a/tests/qunit/ext.popups/actions.test.js
+++ b/tests/qunit/ext.popups/actions.test.js
@@ -1,8 +1,8 @@
-( function ( mw ) {
+( function ( mw, $ ) {
QUnit.module( 'ext.popups/actions' );
- QUnit.test( '#boot', 1, function ( assert ) {
+ QUnit.test( '#boot', function ( assert ) {
var isUserInCondition = function () {
return false;
},
@@ -10,6 +10,8 @@
generateToken = function () {
return '9876543210';
};
+
+ assert.expect( 1 );
assert.deepEqual(
mw.popups.actions.boot( isUserInCondition, sessionID,
generateToken ),
@@ -22,4 +24,91 @@
);
} );
-}( mediaWiki ) );
+ QUnit.module( 'ext.popups/actions#linkDwell @integration', {
+ setup: function () {
+ var that = this;
+
+ this.el = $( '<a>' )
+ .data( 'page-previews-title', 'Foo' )
+ .eq( 0 );
+
+ that.state = {};
+ that.getState = function () {
+ return that.state;
+ };
+ }
+ } );
+
+ QUnit.test( '#linkDwell', function ( assert ) {
+ var done,
+ dispatch = this.sandbox.spy(),
+ gatewayDeferred = $.Deferred(),
+ gateway = function () {
+ return gatewayDeferred;
+ };
+
+ done = assert.async();
+
+ mw.popups.actions.linkDwell( this.el, gateway )( dispatch,
this.getState );
+
+ assert.ok( dispatch.calledWith( {
+ type: 'LINK_DWELL',
+ el: this.el
+ } ) );
+
+ // Stub the state tree being updated.
+ this.state.preview = {
+ activeLink: this.el
+ };
+
+ // ---
+
+ setTimeout( function () {
+ var fetchThunk,
+ result = {};
+
+ assert.strictEqual(
+ dispatch.callCount,
+ 2,
+ 'The fetch action is dispatched after 500 ms'
+ );
+
+ fetchThunk = dispatch.secondCall.args[0];
+ fetchThunk( dispatch );
+
+ assert.ok( dispatch.calledWith( {
+ type: 'FETCH_START',
+ title: 'Foo'
+ } ) );
+
+ gatewayDeferred.resolve( result );
+
+ assert.ok( dispatch.calledWith( {
+ type: 'FETCH_END',
+ result: result
+ } ) );
+
+ done();
+ }, 500 );
+ } );
+
+ QUnit.test( '#linkDwell doesn\'t dispatch the fetch action if the
active link has changed', function ( assert ) {
+ var done,
+ dispatch = this.sandbox.spy();
+
+ done = assert.async();
+
+ mw.popups.actions.linkDwell( this.el /*, gateway */ )(
dispatch, this.getState );
+
+ this.state.preview = {
+ activeLink: undefined // Any value other than this.el.
+ };
+
+ setTimeout( function () {
+ assert.strictEqual( dispatch.callCount, 1 );
+
+ done();
+ }, 500 );
+ } );
+
+}( mediaWiki, jQuery ) );
diff --git a/tests/qunit/ext.popups/processLinks.test.js
b/tests/qunit/ext.popups/processLinks.test.js
index 8059ed6..8f14c53 100644
--- a/tests/qunit/ext.popups/processLinks.test.js
+++ b/tests/qunit/ext.popups/processLinks.test.js
@@ -124,7 +124,7 @@
$.each( cases, function ( i, testCase ) {
assert.strictEqual(
- $processedLinks.eq( i ).data(
'previews-page-title' ),
+ $processedLinks.eq( i ).data(
'page-previews-title' ),
testCase[1]
);
} );
--
To view, visit https://gerrit.wikimedia.org/r/321922
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0b1cf3337a6f8d6450ad2bd127cb292ebb73af4f
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Phuedx <[email protected]>
Gerrit-Reviewer: Phuedx <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
