Phuedx has uploaded a new change for review.
https://gerrit.wikimedia.org/r/322155
Change subject: reducers: Add the nextState helper function
......................................................................
reducers: Add the nextState helper function
OO.copy doesn't copy Element instances, whereas $.extend does. However,
OO.copy does copy properties whose values are undefined or null, whereas
$.extend doesn't.
Since the state tree contains an Element instance - the
preview.activeLink property - we need to use $.extend.
Add the nextState helper function which copies the current state tree
with $.extend and mixes in all updates manually.
Change-Id: Ie8edd9fa0cc3a62a792ed60b49288f85b3ca73e9
---
M resources/ext.popups/reducers.js
M tests/qunit/ext.popups/reducers.test.js
2 files changed, 34 insertions(+), 19 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups
refs/changes/55/322155/1
diff --git a/resources/ext.popups/reducers.js b/resources/ext.popups/reducers.js
index 3a840b2..9592223 100644
--- a/resources/ext.popups/reducers.js
+++ b/resources/ext.popups/reducers.js
@@ -2,6 +2,32 @@
mw.popups.reducers = {};
/**
+ * Creates the next state tree from the current state tree and some
updates.
+ *
+ * In [change listeners](/doc/change_listeners.md), for example, we
talk about
+ * the previous state and the current state (the `prevState` and `state`
+ * parameters, respectively). Since
+ * [reducers](http://redux.js.org/docs/basics/Reducers.html) take the
current
+ * state and an action and make updates, "next state" seems appropriate.
+ *
+ * @param {Object} state
+ * @param {Object} updates
+ * @return {Object}
+ */
+ function nextState( state, updates ) {
+ var result = $.extend( {}, state ),
+ key;
+
+ for ( key in updates ) {
+ if ( updates.hasOwnProperty( key ) ) {
+ result[key] = updates[key];
+ }
+ }
+
+ return result;
+ }
+
+ /**
* Reducer for actions that modify the state of the preview model
*
* @param {Object} state before action
@@ -26,49 +52,42 @@
switch ( action.type ) {
case mw.popups.actionTypes.BOOT:
- // FIXME: $.extend doesn't copy properties
whose values are null or
- // undefined. If we were to do the following:
- //
- // return $.extend( {}, state, { /* ... */ }
);
- //
- // Then the shape of the state tree would vary.
Is this necessarily bad?
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
enabled: action.isUserInCondition,
sessionToken: action.sessionToken,
pageToken: action.pageToken
} );
case mw.popups.actionTypes.LINK_DWELL:
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
activeLink: action.el,
interactionStarted:
action.interactionStarted,
isDelayingFetch: true,
linkInteractionToken:
action.linkInteractionToken
} );
case mw.popups.actionTypes.LINK_ABANDON:
- return $.extend( OO.copy( state ), {
- previousActiveLink: state.activeLink,
+ return nextState( state, {
activeLink: undefined,
interactionStarted: undefined,
- isDelayingFetc: false,
+ isDelayingFetch: false,
linkInteractionToken: undefined
} );
case mw.popups.actionTypes.LINK_CLICK:
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
isDelayingFetch: false
} );
case mw.popups.actionTypes.FETCH_START:
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
isDelayingFetch: false,
isFetching: true,
fetchResponse: undefined
} );
case mw.popups.actionTypes.FETCH_END:
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
isFetching: false,
fetchResponse: action.response
} );
case mw.popups.actionTypes.FETCH_FAILED:
- return $.extend( OO.copy( state ), {
+ return nextState( state, {
isFetching: false,
fetchResponse: action.response // To
catch error data if it exists
} );
diff --git a/tests/qunit/ext.popups/reducers.test.js
b/tests/qunit/ext.popups/reducers.test.js
index f5e0e71..c6d3315 100644
--- a/tests/qunit/ext.popups/reducers.test.js
+++ b/tests/qunit/ext.popups/reducers.test.js
@@ -48,10 +48,6 @@
enabled: true,
sessionToken: '0123456789',
pageToken: '9876543210',
- linkInteractionToken: undefined,
- activeLink: undefined,
- previousActiveLink: undefined,
- interactionStarted: undefined,
isDelayingFetch: false,
isFetching: false
},
--
To view, visit https://gerrit.wikimedia.org/r/322155
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie8edd9fa0cc3a62a792ed60b49288f85b3ca73e9
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Popups
Gerrit-Branch: mpga
Gerrit-Owner: Phuedx <samsm...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
