Jdlrobson has uploaded a new change for review. https://gerrit.wikimedia.org/r/315862
Change subject: WIP: Add `hover` and `displayed` events ...................................................................... WIP: Add `hover` and `displayed` events Although we don't log them on the server they are important stages in the hovercard lifecycle. This introduces them and uses them to track state inside ext.popups.core The benefits of this are that only one event can be in progress at a time. The previous method by making use of data attributes risks logging data from previous hovers or other hovercard interactions when two links are in close proximity. Change-Id: I694b77520d7456483c35facab8321a831a7a2638 --- M resources/ext.popups.core.js M resources/ext.popups.renderer.article.js M resources/ext.popups.renderer/desktopRenderer.js M resources/ext.popups.schemaPopups.utils.js M resources/ext.popups.targets/desktopTarget.js 5 files changed, 30 insertions(+), 33 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Popups refs/changes/62/315862/1 diff --git a/resources/ext.popups.core.js b/resources/ext.popups.core.js index 70520a6..c262209 100644 --- a/resources/ext.popups.core.js +++ b/resources/ext.popups.core.js @@ -1,6 +1,7 @@ ( function ( $, mw ) { 'use strict'; var DWELL_EVENTS_MIN_INTERACTION_TIME, + interactionStartTime, token, perceivedWait, previewCountStorageKey = 'ext.popups.core.previewCount', popupsEnabledStorageKey = 'mwe-popups-enabled'; @@ -50,12 +51,14 @@ * @method log * @param {string} action that is being logged * @param {Object} [logData] - * @param {string} [interactionStartTime] the time the interaction began */ - function shouldLog( action, logData, interactionStartTime ) { - if ( action === 'dwelledButAbandoned' && + function shouldLog( action, logData ) { + // we don't log certain events to the server but we track them internally. + if ( [ 'hover', 'displayed' ].indexOf( action ) > -1 ) { + return false; + } else if ( action === 'dwelledButAbandoned' && interactionStartTime && - logData.linkInteractionToken && + token && mw.now() - interactionStartTime < DWELL_EVENTS_MIN_INTERACTION_TIME ) { return false; @@ -70,17 +73,23 @@ * @method log * @param {string} action that is being logged * @param {Object} [logData] with a set of key/value pairs to be logged in EventLogging. - * callers are advised not to make use of this where they can - * note that a dwellStartTime key should be provided to calculate totalInteractionTime - * @param {string} [interactionStartTime] the time the interaction began */ - mw.popups.log = function ( action, logData, interactionStartTime ) { + mw.popups.log = function ( action, logData ) { logData = logData || {}; + if ( action === 'hover' ) { + interactionStartTime = mw.now(); + token = mw.popups.getRandomToken(); + } + if ( action === 'displayed' ) { + perceivedWait = Math.round( mw.now() - interactionStartTime ); + } if ( shouldLog( action, logData, interactionStartTime ) ) { mw.track( 'ext.popups.schemaPopups', $.extend( {}, logData, { action: action, + linkInteractionToken: token, + perceivedWait: perceivedWait, // Some events e.g. pageLoaded do not have an interaction time. totalInteractionTime: interactionStartTime ? Math.round( mw.now() - interactionStartTime ) : undefined } ) diff --git a/resources/ext.popups.renderer.article.js b/resources/ext.popups.renderer.article.js index ce73a2d..b38527d 100644 --- a/resources/ext.popups.renderer.article.js +++ b/resources/ext.popups.renderer.article.js @@ -71,8 +71,7 @@ mw.popups.log( 'error', $.extend( logData, { errorState: textStatus - } ), - logData.dwellStartTime + } ) ); } deferred.reject(); @@ -553,7 +552,7 @@ mw.popups.settings.open( $.extend( {}, logData ) ); - mw.popups.log( 'tapped settings cog', logData, logData.dwellStartTime ); + mw.popups.log( 'tapped settings cog', logData ); } ); if ( !flippedY && !tall && cache.settings.thumbnail.height < SIZES.landscapeImage.h ) { diff --git a/resources/ext.popups.renderer/desktopRenderer.js b/resources/ext.popups.renderer/desktopRenderer.js index 37f1d53..17e0b03 100644 --- a/resources/ext.popups.renderer/desktopRenderer.js +++ b/resources/ext.popups.renderer/desktopRenderer.js @@ -31,14 +31,14 @@ * @param {Object} event */ function logClickAction( event ) { - mw.popups.log( mw.popups.getAction( event ), logData, logData.dwellStartTime ); + mw.popups.log( mw.popups.getAction( event ), logData ); } /** * Logs when a popup is dismissed */ function logDismissAction() { - mw.popups.log( 'dismissed', logData, logData.dwellStartTime ); + mw.popups.log( 'dismissed', logData ); } /** * @class mw.popups.render @@ -93,10 +93,8 @@ * @method render * @param {jQuery.Object} $link that a hovercard should be shown for * @param {jQuery.Event} event that triggered the render - * @param {number} dwellStartTime the instant when the link is dwelled on - * @param {string} linkInteractionToken random token representing the current interaction with the link */ - mw.popups.render.render = function ( $link, event, dwellStartTime, linkInteractionToken ) { + mw.popups.render.render = function ( $link, event ) { var linkHref = $link.attr( 'href' ), $activeLink = getActiveLink(); @@ -128,9 +126,7 @@ // Set the log data only after the current link is set, otherwise, functions like // closePopup will use the new log data when closing an old popup. logData = { - pageTitleHover: mw.popups.getTitle( linkHref ), - dwellStartTime: dwellStartTime, - linkInteractionToken: linkInteractionToken + pageTitleHover: mw.popups.getTitle( linkHref ) }; $link @@ -211,10 +207,10 @@ mw.popups.$popup.html( mw.popups.$popup.html() ); // Event logging + mw.popups.log( 'displayed' ); $.extend( logData, { pageTitleHover: cache.settings.title, - namespaceIdHover: cache.settings.namespace, - perceivedWait: Math.round( mw.now() - logData.dwellStartTime ) + namespaceIdHover: cache.settings.namespace } ); cache.process( link, $.extend( {}, logData ) ); @@ -364,7 +360,7 @@ function leaveInactive() { var $activeLink = getActiveLink(); - mw.popups.log( 'dwelledButAbandoned', logData, logData.dwellStartTime ); + mw.popups.log( 'dwelledButAbandoned', logData ); // TODO: should `blur` also be here? $activeLink.off( 'mouseleave', leaveInactive ); if ( openTimer ) { diff --git a/resources/ext.popups.schemaPopups.utils.js b/resources/ext.popups.schemaPopups.utils.js index ab859df..c61c392 100644 --- a/resources/ext.popups.schemaPopups.utils.js +++ b/resources/ext.popups.schemaPopups.utils.js @@ -93,8 +93,6 @@ */ function getMassagedData( data ) { data.previewCountBucket = mw.popups.getPreviewCountBucket(); - // FIXME: this should not be necessary and handled by `mw.popups.log` - delete data.dwellStartTime; // Figure out `namespaceIdHover` from `pageTitleHover`. if ( data.pageTitleHover && data.namespaceIdHover === undefined ) { data.namespaceIdHover = new mw.Title( data.pageTitleHover ).getNamespaceId(); diff --git a/resources/ext.popups.targets/desktopTarget.js b/resources/ext.popups.targets/desktopTarget.js index b61d03a..7143262 100644 --- a/resources/ext.popups.targets/desktopTarget.js +++ b/resources/ext.popups.targets/desktopTarget.js @@ -22,9 +22,8 @@ mw.popups.log( 'dwelledButAbandoned', { pageTitleHover: $this.attr( 'title' ), - linkInteractionToken: data.linkInteractionToken, hovercardsSuppressedByGadget: data.hovercardsSuppressedByGadget - }, data.dwellStartTime ); + } ); } /** @@ -35,13 +34,11 @@ */ function onLinkHover( event ) { var $link = $( this ), - // Cache the hover start time and link interaction token for a later use eventData = { - dwellStartTime: mw.now(), - linkInteractionToken: mw.popups.getRandomToken(), hovercardsSuppressedByGadget: isNavigationPopupsGadgetEnabled() }; + mw.popups.log( 'hover' ); // Only enable Popups when the Navigation popups gadget is not enabled if ( !eventData.hovercardsSuppressedByGadget && mw.popups.enabled ) { if ( mw.popups.scrolled ) { @@ -49,8 +46,7 @@ } mw.popups.removeTooltips( $link ); - mw.popups.render.render( $link, event, - eventData.dwellStartTime, eventData.linkInteractionToken ); + mw.popups.render.render( $link, event ); } else { $link .off( 'mouseleave.popups blur.popups click.popups' ) @@ -78,9 +74,8 @@ mw.popups.log( action, { pageTitleHover: $this.attr( 'title' ), - linkInteractionToken: data.linkInteractionToken, hovercardsSuppressedByGadget: data.hovercardsSuppressedByGadget - }, data.dwellStartTime ); + } ); if ( action === 'opened in same tab' ) { event.preventDefault(); -- To view, visit https://gerrit.wikimedia.org/r/315862 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I694b77520d7456483c35facab8321a831a7a2638 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/Popups Gerrit-Branch: master Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits