Commit graph

1490 commits

Author SHA1 Message Date
jhobs 0ff40a6532 Fix tooltip interactions
Tooltips are intended to be stripped upon `mouseenter` and `focus`
events and then restored during their corresponding `mouseleave` and
`blur` events. This was broken due to duplication of event registration
and no proper deregistration.

Changes:
 * Rename `mw.popups.removeTooltips` to `mw.popups.removeTooltip` to
   more accurately describe its effect
 * Narrow the scope of `mw.popups.removeTooltip`
 * Add `mw.popups.restoreTooltip`
 * Add `onLinkBlur` to `desktopTarget.js` to handle tooltip restoration
 * Update qunit test to reflect changes to functions
 * Minor hygiene changes regarding event namespaces

Bug: T142723
Change-Id: I776a72e436ac823fdd6b68435d9a042a91c934e5
2016-11-03 13:39:26 -04:00
jdlrobson 4d7d55ec14 Hygiene: Rename getMassagedData to processHovercardEvent
This name is more fitting to the new purpose of this method.

Change-Id: I48f5e9a3ae737a104270699742a96923c92e12e8
2016-10-31 11:25:09 +00:00
jdlrobson 9477fd5fe6 Multiple hover events should not clear dwellStartTime
If a user has hovercards disabled, when they right click a link
this will trigger another hover event which will reset dwellStartTime.
This means when a dwelledButAbandoned event fires shortly afterwards
the totalInteractionTime will not be correct.

To remedy this, the calculation of totalInteractionTime and
perceivedWait is moved into ext.popups.schemaPopups

Now that we can trigger events without logging to the server and
checking the total interaction time duration in two places,
let's always run the event and only check it once.

A `hover` event triggers the setting of a dwell start time
A `display` event triggers the setting of the perceivedWait value
* Both are reset on a dwelledButAbandoned.

Since dwellStartTime is controlled inside a single place, getMassageData
no longer needs to clear it.

The test "returns false if dwelledButAbandoned event without a dwellStartTime"
is removed as this should no longer be possible.

Bug: T147846
Change-Id: Ie5917ca86f0d0ab27f4cf507e6dfa2c271433c03
2016-10-28 17:33:10 +01:00
jdlrobson 47c2df09d4 Display and hover events are logged but not recorded
This adds two new events "display" and "hover" which are not
recorded back to the server. The benefits of having these events
is that they are important events in the lifecycle of a hovercard.

This allows us to debug trackSubscribe and ensure we see the behaviour
we expect to see and in a future patchset will allow us to use these
events to drive the calculation of interaction time in one single location
(Sneak preview:
 https://gerrit.wikimedia.org/r/316481 to get a feel for the why.)

Change-Id: I58eefc29444179fd245cfd722093dedea19455e8
2016-10-25 19:55:35 +00:00
jdlrobson c4460ba2ea Hygiene: Move logic for duplicate events into getMassagedData
Add tests.

Change-Id: I15c10f256432ab5bfa7bf7adb34764f84b17c439
2016-10-25 19:33:29 +00:00
jdlrobson 1f010664ef Hygiene: Rename track topic to be more consistent with new plans
I plan to track events that will not be logged by the schema so
am dropping the 'schema' from the topic name.

This will allow us to track events that we do not log via the schema

Change-Id: I0c2762f7ed6e54fff765513b2c2d32f73fe8902f
2016-10-25 20:00:18 +01:00
jenkins-bot fcef6d2fff Merge "Tests: Test existing behaviour for getMassagedData" 2016-10-25 13:54:45 +00:00
Chad Horohoe 39ca253530 Whoops, track not trace
Change-Id: I9f3443e34502eca326dc5bcacfc176a088e6b57c
2016-10-24 17:03:20 -07:00
Chad Horohoe 31451e6659 Swapping defaultbranch for trace
The former is a maintenance nightmare when branching.

Bug:T146293
Change-Id: I8a542487a6cc61b6c8933280ce2bd0c8bb6ec021
2016-10-24 16:38:10 -07:00
jdlrobson 8fb7e6fbe6 Tests: Test existing behaviour for getMassagedData
Change-Id: Ia26600222c02b986a05918a00c0776c7dc8deb77
2016-10-24 15:52:21 -07:00
Baha cd0546beb8 Fix browser tests
The Hovercards checkbox on the beta preferences page doesn't have an ID
anymore. It has a name though, which is used from now on.

The regression was introduced in I8636f32330e23814ba3b4c0f5e22e55aaf77883e.

Bug: T148856
Change-Id: I7baa78b0d8c560ee29c3a550628bfd47cad1c30e
2016-10-24 14:27:09 -04:00
jdlrobson 74231eba4a Hygiene: Remove dead code
Since DWELL_EVENTS_MIN_INTERACTION_TIME is always
zero the check will always be true so let's not make
that check unnecessarily.

We might change this in future is not a good reason
for keeping this code.

Change-Id: I23cb7ef9caeb3df470ccf109b815c4c566b0a735
2016-10-19 13:57:16 -07:00
jdlrobson 7dfad14da1 Directory structure should reflect the ResourceLoader definitions
Given we currently have modules defined in extension.json and in hooks
it can be really confusing understanding how the code fits together.

This change hopefully makes this a little clearer by using folder names
that are named after the resource loader modules - this is also consistent
with how we do things in our other extensions.

A images folder is added to the route so that it is clearer that the images
are not used in ResourceLoader module definitions and are only used to illustrate
the beta feature.

Change-Id: Ia650ec03e3a6d3069165441ddfa069d390be4d10
2016-10-19 20:52:40 +00:00
jenkins-bot 04c09cd067 Merge "Hygiene: Pull out event handler for display of Hovercards" 2016-10-19 10:51:15 +00:00
Translation updater bot 7b2528c600 Localisation updates from https://translatewiki.net.
Change-Id: Id3fa589c63c04d333d14965e14320a2caac1e346
2016-10-17 22:34:56 +02:00
jdlrobson bf8485b6c7 Hygiene: Pull out event handler for display of Hovercards
* Make it clearer what event is being bound, consistent with
onLinkClick and onLinkAbandon
* Don't create event data twice
* Refactor to reduce function size

Change-Id: Ie4368531612a2829ad191629410ba548eadb2007
2016-10-13 15:59:29 -07:00
Ed Sanders 469d3e7f02 Update beta feature screenshots to use proper layout and colours
Bug: T144428
Change-Id: Ic97bed840c5e9c34a4c8e81a2148e2a0af4ed286
2016-10-12 15:32:21 -07:00
jdlrobson aa5e622ca0 Event handlers should not be exposed globally
Bug: T147306
Change-Id: Ib0bae3e7c2e420a4b717391d4a25473a3a9f9128
2016-10-12 14:58:17 -07:00
Translation updater bot 945920f33e Localisation updates from https://translatewiki.net.
Change-Id: Iecd06481e59285c6ccec8e4ad85c968c1869a768
2016-10-11 22:47:51 +02:00
Translation updater bot fe66ae3940 Localisation updates from https://translatewiki.net.
Change-Id: I7bf5458880af0e54bb9eda99949032438bfc8663
2016-10-05 22:44:01 +02:00
jenkins-bot 46274a5518 Merge "Regression: correctly log the click action total interaction time" 2016-10-04 10:23:46 +00:00
jenkins-bot fcde4eaed5 Merge "EventLogging sampling rate should default to 0" 2016-10-04 10:18:21 +00:00
jenkins-bot 66d8daddba Merge "Regression: Correctly log total interaction time for dismiss event" 2016-10-03 11:09:52 +00:00
Baha 3c0ea0f9cd Regression: correctly log the click action total interaction time
Per T143051, hovercards was asked to be closed after the user clicks on
the hovercard (or the link that showed it). Closing the popup before
logging the `click` event causes `logData` to be lost. This patch logs
the data first and then clears it.

The regression was introduced in Ifd6f75c2a53d8d7b5ef9fd3f232f85b55eea24c8.

Also do not attempt closing a popup when it may not be open.

Bug: T146934
Change-Id: I02febc83036130bcea0a769114c9126cb481bafe
2016-09-30 12:03:09 +00:00
Baha ae96fddb40 Regression: Correctly log total interaction time for dismiss event
Per T143051, hovercards was asked to be closed after the user clicks on
the hovercard (or the link that showed it). Doing so before logging the
`dismiss` event caused `logData` to be lost. This patch logs the data first
and the clears it.

The regression was introduced in I3f3c4780cc31dc8d84cdd76df2c77fa45fbea882

Bug: T146927
Change-Id: Ie965d43b04962b2dbe2d15caa4c14bf62d1e39ea
2016-09-30 08:02:05 -04:00
Prateek Saxena 898648dfa2 settings: Use mw-ui-progressive instead of progressive buttons
Bug: T146923
Change-Id: I35078d5f61afb368ff3fc37e2f9cf0986bed2294
2016-09-29 18:47:00 +05:30
jdlrobson f974a024f4 Clicking a Hovercards legible link should close popup
A click event is used to capture links that may be clicked via
the keyboard.

Bug: T143051
Change-Id: Ifd6f75c2a53d8d7b5ef9fd3f232f85b55eea24c8
2016-09-23 21:01:50 +00:00
jdlrobson 9f92324b4d Dismiss event should not be tied to render method
The dismiss event is currently a side effect of closing a popup.
This should be more closely coupled with the event that causes it.

This allows the closePopup method to be called without trigging
an event which is a necessary precursor to closing popups when
navigating away from the page.

Change-Id: I3f3c4780cc31dc8d84cdd76df2c77fa45fbea882
2016-09-23 20:56:46 +00:00
jdlrobson 25fabbf5f4 Hygiene: Only allow calls to closePopup on an active link
Change-Id: I7c15870e1586aa8629af723accad67dee5483369
2016-09-23 20:27:41 +00:00
jdlrobson 5a226ebd39 Hygiene: Add set and get methods for active link
Named functions help explain to a reader and reviewer what the code
is actually doing.

Change-Id: I1d059c9270fd2298285fa5e4e52e403a06f35503
2016-09-23 11:01:21 -07:00
jdlrobson da5d61e485 Improve documentation for render methods
The parameter types are incorrect and this leads to confusion
Use $ prefix for $link parameter to make clear it is a jQuery.Object

Change-Id: I3f98f3729cd06aedd791e7503233082c1402dc95
2016-09-22 15:28:57 -07:00
jenkins-bot 5520c6c4ac Merge "Merge mw.popups.experiment into mw.popups.core" 2016-09-21 22:31:39 +00:00
jenkins-bot 98d76e2591 Merge "Minor refactor - use named function for wikipage.content hook" 2016-09-21 21:34:28 +00:00
jdlrobson 523213466e Minor refactor - use named function for wikipage.content hook
The handler for the hook `wikipage.content` was long
and undocumented but is a crucial piece of code.

To make it a little easier for people not so familiar with the code
to find this code and understand what it does I've pulled it out into
a named function.

Change-Id: I3ada53e135dea7dc6846440999b0d42285e14013
2016-09-21 14:23:25 -07:00
Sam Smith 0b2961c318 Merge mw.popups.experiment into mw.popups.core
ext.popups.experiment depends on .core as it initialized the mw.popups
namespace and .core depends on .experiments for
mw.popups#getEnabledState.

By merging the experiment module into core, we can eliminate any
circular dependencies.

Changes:
* Move ext.popups.experiment.js code into ext.popups.core.js
* Remove mw.popups.experiment module and any references to it

Note: ext.popups.experiment.test.js was left in its own file for cleaner
QUnit module setups and easier removal later. I'm not entirely happy
with doing it this way, but I'm not sure changing the mw.config within
the mw.popups.core QUnit module is worth merging the files.

Bug: T146035
Change-Id: I1f024567010acaa61c1d613c6e59c998198a5976
2016-09-21 16:43:49 -04:00
joakin 8f001ee4bd Respect beta feature flag for logged in users
Both in active and inactive state (not just active like before).

Bug: T146017
Change-Id: Ieb4cd39c2870fb701cabd0f1d01c0ab42b6ebe78
2016-09-20 17:48:39 +02:00
jhobs c9c82828d0 EventLogging sampling rate should default to 0
The EventLogging sampling rate should be set in the wiki config.

Bug: T136746
Change-Id: I7a954edf00e56b426f94f39d39c19cbc338186d6
2016-09-16 12:27:58 -04:00
joakin d0d10f51bb Update Schema to the last version
Which has updated docs about the min dwell interaction time.

Everything else stays the same.

See https://meta.wikimedia.org/wiki/Schema:Popups

Bug: T145379
Change-Id: Iae5da98cfc3f2290aa93aa04effab016b5d700b4
2016-09-14 14:52:33 -04:00
joakin 9787f515d0 Log all dwelledButAbandoned events
Extract the minimum interaction time of dwelledButAbandoned events to
a unique place: mw.popups.render.DWELL_EVENTS_MIN_INTERACTION_TIME

Lower that interaction time to 0 so that all dwelledButAbandoned events
are logged.

Bug: T145379
Change-Id: Id68183367966b9f0d52e0cd570cab64671a3e87e
2016-09-14 14:50:39 -04:00
joakin eb852cacd5 Fix enabling/disabling browser tests
features/popups_settings.feature:33 seems to fail because of quick
interactions after disabling+enabling hovercards.

Setting a sleep 1 after disabling and enabling hovercards (to allow the
page to start refreshing) makes the tests more resilient

Bug: T145152
Change-Id: Iaf66aaf0e2c4f8e55419d22889f91a02891e49a1
2016-09-14 16:42:56 +00:00
Alex Monk 5574c98628 Follow-up I6dac2911: ext.popups.core depends on mediawiki.storage
To stop TypeError: Cannot read property 'get' of undefined in
Object.mw.popups.getEnabledState

Change-Id: I8fb648e1e6a4921ba04752a4314cb5937485ac01
2016-09-08 17:40:01 +01:00
Translation updater bot 5180dd6de3 Localisation updates from https://translatewiki.net.
Change-Id: I99980fd55da0e826c25fd83a1e51e8229c57792b
2016-09-07 22:45:35 +02:00
Translation updater bot 1a708bd6db Localisation updates from https://translatewiki.net.
Change-Id: I1f6c36f0939d89ae2e8949fbf62b3bf1b50fca69
2016-09-04 00:23:59 +02:00
Translation updater bot 010b0417a8 Localisation updates from https://translatewiki.net.
Change-Id: If89d2beaae917e4488010f41462fbea241f1fb30
2016-09-01 23:26:35 +02:00
jenkins-bot 994b14c16b Merge "Do not log the click event multiple times" 2016-08-31 10:29:32 +00:00
Translation updater bot 023943eb8a Localisation updates from https://translatewiki.net.
Change-Id: Ib0e278269b8be88671ad06851a4a1c1273349221
2016-08-29 22:38:41 +02:00
Baha 1d71b3d9bd Do not log the click event multiple times
When hovercards are off clicking of a link will log the click
event as many times as you hovered over the link prior to clicking.
To prevent this, stop listening to old events before listening to
new ones.

Bug: T143805
Change-Id: I3e0dd2d1f259cdadfd3e02c67a137697540ca955
2016-08-29 14:33:21 -04:00
Translation updater bot fb0306469f Localisation updates from https://translatewiki.net.
Change-Id: I4bcf2a8b62dd041a8df58aa5e0d0e6bffe342410
2016-08-28 22:40:43 +02:00
Translation updater bot 004c645ea3 Localisation updates from https://translatewiki.net.
Change-Id: I5479769f1ddae50be5cf209dada6df32c4b63e6e
2016-08-25 22:38:22 +02:00
Translation updater bot ff4281cdb7 Localisation updates from https://translatewiki.net.
Change-Id: Ifb81ddd701006c210a56effdf325843cc4d89302
2016-08-24 22:38:10 +02:00