Commit graph

852 commits

Author SHA1 Message Date
Sam Smith 16adb1d738 Remove optional survey link
Change-Id: If545873bb97b1da8ea5001dce575b1ef512903fb
2016-11-09 14:41:03 +00:00
Sam Smith 19349c0108 Add BOOT action
Changes:
* Make grunt:qunit run all QUnit tests in those modules whose names
  being with "ext.popups".
* Add ext.popups/index.js, which initialises the mw.popups namespace.
* Add ext.popups/userSettings.js, which contains the code that deals
  with interacting with the User Agent's storage.
* Add ext.popups/experiment.js, which contains the code that that
  decides whether or not the user is in the experiment condition.
* Add tests for both units, converting existing tests where appropriate.
* Remove the associated code from ext.popups.core/ext.popups.core.js.
* Finally, dispatch the BOOT action against the store in
  ext.popups/boot.js.

Change-Id: I697207677304bd49c7cfe1d37bb0a4af7810f387
2016-11-09 10:40:59 +00:00
Sam Smith 83dbf746ba Enable Redux DevTools in debug mode
Also, fix a bug introduced in Ib7168217 wherein the non-existent
Redux.thunk was referenced rather than ReduxThunk.default.

Change-Id: Ia4cc28b16b17442de69ed84bb8e8c88a6a9f201d
2016-11-09 10:38:20 +00:00
jhobs 9d22bc8bd8 Make mpga the default branch for git-review
Git should probably push changes ON the mpga branch TO the mpga
branch...

Change-Id: I6e047653c14a0d9bb40f09dcb0e44c4d1ffe5308
2016-11-08 21:00:21 +00:00
Sam Smith e47fe4d04d Use Redux and Redux Thunk
Changes:
* Create the ext.popups.lib module, which contains redux@3.6.0 and
  redux-thunk@2.1.0.
* Rely on the Resource Loader's minification and mangling (?)
  mechanisms.
* Create an asynchronous bootstrap script, which creates a Redux store
  when the User Agent is idle.

Change-Id: Ib7168217a5673bb2a8378eb30d6aa45043c66e62
2016-11-08 15:33:20 -05:00
Sam Smith 3bf0f8ab8b Remove ResourceLoaderRegisterModules hook handler
We should endeavour to define Resource Loader modules in one place,
extension.json. Adding them to the output, however...

Changes:
* Remove ResourceLoaderRegisterModules hook handler.
* Move all dependencies, messages, templates, and styles into "empty" RL
  module definitions in extension.json so that they don't have to be
  looked up later.

Change-Id: I7ec963e3296de8f411e90079ab13b97c6eba4916
2016-11-08 14:14:16 -05:00
jhobs 046d12f51b Update linting and enable command line QUnit
You can use `grunt watch` now!

Changes:
 * Re-enable QUnit (but ignore legacy tests)
 * Add a sample QUnit test
 * Add ability to run QUnit via `grunt watch` or `grunt test`
 * Move linting to `grunt lint` task
 * Add Redux to globals in linter

Change-Id: Ie4a65a8a66773d6472b3d73257267d18027ff3c3
2016-11-08 13:56:17 -05:00
Sam Smith 89975c4942 Remove mobile support
Note well that mobile support is a consideration in the refactoring
effort (see T149801).

Change-Id: I82adc1550b66d0a456aa299ad6c8d2d097fbb7ae
2016-11-08 09:32:04 +00:00
Sam Smith f64c6ea509 Disable QUnit and acceptance tests
Change-Id: Icee115520ec559f8e107de2946224ac1fe65154d
2016-11-08 09:31:51 +00:00
Translation updater bot 6cacefcfd9 Localisation updates from https://translatewiki.net.
Change-Id: I48ed4406c244ac1a6d79aebdce5fd11ee424074c
2016-11-05 22:56:56 +01:00
Jhobs 102d02b891 Revert "Revert "Fix tooltip interactions""
This reverts commit 11431dd2f6.
It also applies the fix for the original error.

Change-Id: Ib86534ffbcd20f64c8ba06c23f2e8af509437cfe
2016-11-03 13:51:36 -04:00
Jdlrobson 11431dd2f6 Revert "Fix tooltip interactions"
There is one reference to removeTooltips in 
ext.popups.targets.desktopTarget.

I suspect this got broken in a rebase. It seems Jenkins only runs
browser tests post merge. We should fix that.

Please resubmit the patch with this amended.

This reverts commit 0ff40a6532.

Change-Id: Idd8dffb853db760ebc5866190d008f173e3025ba
2016-11-03 17:46:41 +00:00
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