mirror of
https://gerrit.wikimedia.org/r/mediawiki/extensions/Popups
synced 2024-11-23 23:24:39 +00:00
Clear interaction after an event for it is logged in EL
Given that interactions end up with an event logged, there shouldn't be a reason to keep an interaction active after it's corresponding final event has been logged. See Tbayer's state graph. This patch removes the current interaction if an event with that token is logged, effectively finalizing it and making it impossible for the token to be reused from the state tree again. Additional changes: * Pass the logged event with the action EVENT_LOGGED so that the reducer can determine if it needs to do anything else. * Since the interaction is removed, when undefined, guard against actions that use state.interaction freely. (Only allow BOOT, LINK_PREVIEW, and EVENT_LOGGED) Bug: T161769 Bug: T163198 Change-Id: I99fd5716dc17da32929b6e8ae4aa164f9d84c387
This commit is contained in:
parent
cf0ea9db7b
commit
293d7ebe8d
BIN
resources/dist/index.js
vendored
BIN
resources/dist/index.js
vendored
Binary file not shown.
BIN
resources/dist/index.js.map
vendored
BIN
resources/dist/index.js.map
vendored
Binary file not shown.
|
@ -307,11 +307,13 @@ actions.saveSettings = function ( enabled ) {
|
|||
* Represents the queued event being logged `changeListeners/eventLogging.js`
|
||||
* change listener.
|
||||
*
|
||||
* @param {Object} event
|
||||
* @return {Object}
|
||||
*/
|
||||
actions.eventLogged = function () {
|
||||
actions.eventLogged = function ( event ) {
|
||||
return {
|
||||
type: types.EVENT_LOGGED
|
||||
type: types.EVENT_LOGGED,
|
||||
event: event
|
||||
};
|
||||
};
|
||||
|
||||
|
|
|
@ -92,10 +92,14 @@ module.exports = function ( boundActions, schema, track ) {
|
|||
|
||||
hashToSeenMap[ hash ] = true;
|
||||
|
||||
event = $.extend( true, {}, eventLogging.baseData, event );
|
||||
|
||||
if ( shouldLog ) {
|
||||
schema.log( $.extend( true, {}, eventLogging.baseData, event ) );
|
||||
schema.log( event );
|
||||
}
|
||||
|
||||
boundActions.eventLogged();
|
||||
// Dispatch the eventLogged action even if it was a duplicate so that the
|
||||
// state tree can be cleared/updated.
|
||||
boundActions.eventLogged( event );
|
||||
};
|
||||
};
|
||||
|
|
|
@ -123,7 +123,7 @@ function createClosingEvent( interaction ) {
|
|||
* current state
|
||||
*/
|
||||
module.exports = function ( state, action ) {
|
||||
var nextCount,
|
||||
var nextCount, newState,
|
||||
actionTypesWithTokens = [
|
||||
actionTypes.FETCH_COMPLETE,
|
||||
actionTypes.ABANDON_END,
|
||||
|
@ -148,6 +148,22 @@ module.exports = function ( state, action ) {
|
|||
return state;
|
||||
}
|
||||
|
||||
// If there is no interaction ongoing, ignore all actions except for:
|
||||
// * Application initialization
|
||||
// * New link dwells (which start a new interaction)
|
||||
// * Clearing queued events
|
||||
//
|
||||
// For example, after ctrl+clicking a link or preview, any other actions
|
||||
// until the new interaction should be ignored.
|
||||
if (
|
||||
!state.interaction &&
|
||||
action.type !== actionTypes.BOOT &&
|
||||
action.type !== actionTypes.LINK_DWELL &&
|
||||
action.type !== actionTypes.EVENT_LOGGED
|
||||
) {
|
||||
return state;
|
||||
}
|
||||
|
||||
switch ( action.type ) {
|
||||
case actionTypes.BOOT:
|
||||
return nextState( state, {
|
||||
|
@ -159,10 +175,24 @@ module.exports = function ( state, action ) {
|
|||
} );
|
||||
|
||||
case actionTypes.EVENT_LOGGED:
|
||||
return nextState( state, {
|
||||
newState = nextState( state, {
|
||||
event: undefined
|
||||
} );
|
||||
|
||||
// If an event was logged with an interaction token, and it is still
|
||||
// the current interaction, finish the interaction since logging is
|
||||
// the exit point of the state machine and an interaction should never
|
||||
// be logged twice.
|
||||
if (
|
||||
action.event.linkInteractionToken &&
|
||||
state.interaction &&
|
||||
( action.event.linkInteractionToken === state.interaction.token )
|
||||
) {
|
||||
newState.interaction = undefined;
|
||||
}
|
||||
|
||||
return newState;
|
||||
|
||||
case actionTypes.FETCH_COMPLETE:
|
||||
return nextState( state, {
|
||||
interaction: nextState( state.interaction, {
|
||||
|
|
|
@ -75,6 +75,9 @@ QUnit.test( 'it should call the eventLogged bound action creator', function ( as
|
|||
this.changeListener( undefined, state );
|
||||
|
||||
assert.ok( this.boundActions.eventLogged.called );
|
||||
assert.deepEqual( this.boundActions.eventLogged.getCall( 0 ).args[ 0 ], {
|
||||
action: 'pageLoaded'
|
||||
} );
|
||||
} );
|
||||
|
||||
QUnit.test( 'it should handle duplicate events', function ( assert ) {
|
||||
|
|
|
@ -96,7 +96,8 @@ QUnit.test( 'EVENT_LOGGED', function ( assert ) {
|
|||
};
|
||||
|
||||
action = {
|
||||
type: 'EVENT_LOGGED'
|
||||
type: 'EVENT_LOGGED',
|
||||
event: {}
|
||||
};
|
||||
|
||||
assert.deepEqual(
|
||||
|
@ -106,6 +107,28 @@ QUnit.test( 'EVENT_LOGGED', function ( assert ) {
|
|||
},
|
||||
'It dequeues any event queued for logging.'
|
||||
);
|
||||
|
||||
// ---
|
||||
|
||||
state = {
|
||||
interaction: { token: 'asdf' },
|
||||
event: { linkInteractionToken: 'asdf' }
|
||||
};
|
||||
|
||||
action = {
|
||||
type: 'EVENT_LOGGED',
|
||||
event: state.event
|
||||
};
|
||||
|
||||
assert.deepEqual(
|
||||
eventLogging( state, action ),
|
||||
{
|
||||
event: undefined,
|
||||
interaction: undefined
|
||||
},
|
||||
'It destroys current interaction if an event for it was logged.'
|
||||
);
|
||||
|
||||
} );
|
||||
|
||||
QUnit.test( 'PREVIEW_SHOW', function ( assert ) {
|
||||
|
@ -658,7 +681,8 @@ QUnit.test( 'ABANDON_END doesn\'t enqueue an event under certain conditions', fu
|
|||
} );
|
||||
|
||||
state = eventLogging( state, {
|
||||
type: 'EVENT_LOGGED'
|
||||
type: 'EVENT_LOGGED',
|
||||
event: {}
|
||||
} );
|
||||
|
||||
state = eventLogging( state, {
|
||||
|
|
Loading…
Reference in a new issue