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

Reply via email to