Commit graph

81 commits

Author SHA1 Message Date
Jeff Hobson 047bccfac1 Settings dialog
Also removes some unused (redundant) actions and the renderer reducer.

Bug: T152223
Change-Id: I878c7f16d71f8cdbd74a47ffeb9dadc4decf2883
2016-12-12 04:54:34 -05:00
jenkins-bot ec90fcba6f Merge "previews: Add generic fallback preview" into mpga 2016-12-02 12:06:53 +00:00
Sam Smith b2f9ffa9b0 previews: Add generic fallback preview
Changes:
* Make the gateway handle missing pages, which are characterised by the
  MediaWiki API response having both the missing property set to truthy
  and the page not having any revisions.
* Add the preview-empty template and associated "mwe-popups-is-empty"
  CSS class, which describe what an empty preview contains and how it
  should look.

Supporting changes:
* Move the original preview template into the ext.popups module.

Bug: T151054
Change-Id: Ife75bf9c6bafdfe0a6cc3e20eea853b4ac8f951b
2016-12-02 12:02:43 +00:00
jenkins-bot 9a7045d3bd Merge "Hygiene: Don't fall through unless necessary" into mpga 2016-12-02 11:24:07 +00:00
Sam Smith 27334437fb Hygiene: Don't fall through unless necessary
The preview reducer would treat the LINK_ABANDON_END and
PREVIEW_ABANDON_END actions as the PREVIEW_DWELL action. Fortunately,
this was a NOOP as they would only fall through if the preview was being
dwelled upon.

HT Joaquin Hernandez.

Change-Id: I01ff47de34ac41f1e6aa46aa5baccc2a27013f1b
2016-12-02 12:01:44 +01:00
jenkins-bot b5da2e1c13 Merge "Tests: Deeply extend prev state for test correctness" into mpga 2016-12-02 10:49:50 +00:00
joakin ac264001f6 Tests: Deeply extend prev state for test correctness
As it is, that test is passing the same previous state and current state
because extend by default only copies one level deep, resulting in the
manual mutation mutating both prev and current state.

This passes because the change listener always sets the link visibility
from the current state regardless of the previous state (it is doing
redundant work, but not too much).

Change-Id: I8cf125cc4f6833fbce0fe14bdcb4ef550c7e8450
2016-11-30 19:22:54 +01:00
joakin 8cd297c797 Hygiene: #processLinks Improve wording on tests
Change-Id: Ib6dd29d2c27dd10525fd9d875bfc860c5b76c841
2016-11-30 17:01:54 +00:00
joakin eae0a96e80 Hygiene: Only use that when necessary. Else use this
Change-Id: Idd91300041c1ae68a24a49626f79e97b3e8b2467
2016-11-30 17:01:35 +00:00
joakin cd7f74f691 Tests: Explain the second assert in the test
Like the next test also does, so that in case it fails we get some info
given the assert result doesn't match the test title.

Change-Id: I42d9e7c329c1a0b46133423e5303068b70b270ac
2016-11-30 17:00:20 +00:00
Sam Smith e4719c4918 Don't hide preview if it's interacted with
Action creator changes:
* Make the linkAbandon action creator asynchronous by splitting it into
  two distinct actions, LINK_ABANDON_START and _END, the latter of which
  is dispatched after a 300 ms delay.
* Introduce the previewDwell and previewAbandon action creators. The
  latter is an asynchronous action that mirrors the linkAbandon action.

Reducer changes:
* Make the LINK_DWELL, LINK_ABANDON_END, and PREVIEW_ABANDON_END action
  hide a preview, if one has been shown.
* Make the LINK_ABANDON_END action NOOP if:
  * The user has interacted with another link, or
  * The user is interacting with the preview.

Supporting changes:
* Update the mw.popups.reducers#preview and #renderer unit tests to use
  an empty previous state so that the tests are more resilient to
  modifications of the state tree.

Change-Id: I2ecf575bbb59bb64772f75da9b5a29c071b46a8d
2016-11-28 17:15:37 +00:00
Sam Smith 1210f2f63e actions: Don't fetch if user isn't in condition
Change-Id: I6884f2da38c5b0addcd0a8b3325363efd46401f5
2016-11-25 21:22:09 +00:00
Sam Smith eabb7011fb Don't always render after the API request resolves
If the user abandons the link after the API request delay (500 ms) but
before the it resolves (~10e3 ms), then the preview shouldn't be
rendered.

Changes:
* actions: Include the link in the FETCH_START, FETCH_FAILED, and
  FETCH_END actions.
* reducers: If the active link has changed, then FETCH_END is a NOOP.

Supporting changes:
* reducers: Signal that a preview should be rendered and shown with
  preview.shouldShow.

Change-Id: I3dd1c0c566ec63de515174c14845d7927583ce93
2016-11-25 12:42:02 +00:00
Sam Smith 582fab6aaf actions: window.setTimeout -> mw.popups.wait
The QUnit test suite now completes in ~10e2 ms rather than ~10e3 ms.

Changes:
* Make mw.popups.actions#linkDwell to use mw.popups.wait.
* Make the tests for mw.popups.actions#linkDwell complete faster, but
  still asynchronously, by stubbing mw.popups.wait.

Change-Id: I5cbef0ea69bc860f75cac27c1adea3d419c1ffad
2016-11-25 10:41:39 +00:00
Sam Smith 0d68a8f635 reducers: Remove unused state and cases
... from the preview reducer.

Change-Id: Ic7b9c38a39e0aec4f67a20b921acde2f9bbf362f
2016-11-25 10:41:04 +00:00
Sam Smith 5e2d8ae8d4 Render previews
Extract core rendering functionality from the mw.popups.renderer and
mw.popups.renderer.article objects.

For now, render and show the preview when the user dwells on and
abandons a link respectively.

Supporting changes:
* Add mw.popups.wait, which is sugar around window.setTimeout.
* action.response -> action.result in the FETCH_END case of the preview
  reducer.

Change-Id: I14b437e7c2f55b988837fcb2800dd61a23c29a01
2016-11-24 18:07:03 +00:00
Sam Smith 1c861fd9de actions: Include event in LINK_DWELL action
Also include the time at which the interaction started.

Change-Id: Ie46562a2641e8bd26fc687e16e4e7ef3760824b1
2016-11-22 13:46:44 +00:00
Sam Smith 587f060569 gateway: Remove parentheticals from extract
Extracted from ext.popups.renderer.article#removeParensFromText.

Change-Id: I200d431234c4b235a8621e1b261934b1282dd8e6
2016-11-21 10:41:45 +00:00
Sam Smith 261ab1799b gateway: Check if page has been revised recently
Extracted from ext.popups.renderer.article#init.

Change-Id: I906d74ded1082c9caf6f1b870309c815a5876c17
2016-11-21 10:32:28 +00:00
Sam Smith b1b29f4704 Link Previews -> Page Previews
Change-Id: I08e80220c76a65cadaba20ce78b9eb2b36139095
2016-11-18 09:51:06 +00:00
Sam Smith 089ee014ad Add link title change listener
Supporting changes:
* Remove the preview.previousActiveLink property from the state tree as
  it's unnecessary.

Change-Id: I657decf9425a7a9e2b27a798ed60b162569661d8
2016-11-18 09:51:01 +00:00
Sam Smith f6868d2567 reducers: Add the nextState helper function
OO.copy doesn't copy Element instances, whereas $.extend does. However,
OO.copy does copy properties whose values are undefined or null, whereas
$.extend doesn't.

Since the state tree contains an Element instance - the
preview.activeLink property - we need to use $.extend.

Add the nextState helper function which copies the current state tree
with $.extend and mixes in all updates manually.

Change-Id: Ie8edd9fa0cc3a62a792ed60b49288f85b3ca73e9
2016-11-17 21:41:56 +00:00
Sam Smith 0d300867ab Make API request after 500 ms
If the user has abandoned the link or dwelled on a different link, then
don't make the API request.

Changes:
* Fix a bug in the linkDwell reducer where the activeLink was always
  being set to undefined.
* Add the private fetch action creator.
* Make the linkDwell action dispatch the result of the fetch action
  creator after 500ms.

Supporting changes:
* Make the link* action creators take DOM elements rather than jQuery
  instances so that:
  1. Testing whether the state tree has changed is an identity
     comparison.
  2. jQuery instances are constructed only when required.
* Document the ext.popups.Gateway type.
* Document the Redux.Store type.
* Rename the "previews-page-title" data attribute to
  "page-previews-title" to avoid confusion.

Change-Id: I0b1cf3337a6f8d6450ad2bd127cb292ebb73af4f
2016-11-17 13:20:11 +00:00
jenkins-bot 156d552fcf Merge "Add footer link change listener" into mpga 2016-11-17 08:12:06 +00:00
Jeff Hobson f9cc341105 Add reducer cases for all actions
These may change as actions are implemented. Also fixes a typo in the
QUnit test for reducers.

Change-Id: I2218760f273c77c5d396177c99108a57de7162d6
2016-11-16 11:16:43 -05:00
Sam Smith a9e78f06ae Add footer link change listener
Supporting changes:
* Add mw.popups.registerChangeListener, which registers a change
  listener that will only be called when the state in the store has
  changed.

Change-Id: Ibe6934058327c7f02f7d8092e74a667a5a1c600a
2016-11-16 15:20:34 +00:00
Sam Smith ca84de7c9d Add tokens to BOOT action
Changes:
* Add sessionToken and pageToken properties to the BOOT action and
  update the preview reducer.

Supporting changes:
* Move the mw.popups.createActions to ext.popups/boot.js so that Redux
  is used in one file and the actions can be tested in isolation more
  easily.

Change-Id: Icd61bf1aeb466899e047432bf9798e2574652830
2016-11-14 19:42:52 +00:00
Jeff Hobson 2215560866 Add reducers
Reducers as a whole are a WIP, but this implements a baseline from which
to add more.

Changes:
 * Create ext.popups.reducers to house all reducers
 * Create reducers for preview and renderer state manipulation
 * Create rootReducer by combining preview and renderer reducers
 * Add QUnit tests for reducers
 * Move action types into ext.popups.actionTypes
 * Extract rootReducer from boot.js

Change-Id: I8a2296c6846cd4b0552a485e671af1d974944195
2016-11-11 19:55:04 +00:00
Sam Smith 722bfe12a5 Add gateway
Supporting changes:
* Automatically register JavaScript files in the
  tests/qunit/ext.popups/ directory.

Additional changes:
* Fix a grammatical error in the docblock for
  mw.popups.createExperiment.

Change-Id: Ieb00709e353f0b960375fdaa0ca0dcdf950f2eb9
2016-11-11 18:59:44 +00:00
Sam Smith 9611d3b2db Add LINK_DWELL and LINK_ABANDON actions
Supporting changes:
* Add mw.popups.processLinks.
* Extract the existing unit tests for mw.popups.selectPopupElements and
  mw.popups.getTitle.
* Fix Grunt QUnit timeout.

Change-Id: I325bcb15abc6e0b745d78b7308a346a034ab2988
2016-11-10 11:47:55 +00:00
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
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
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 8fb7e6fbe6 Tests: Test existing behaviour for getMassagedData
Change-Id: Ia26600222c02b986a05918a00c0776c7dc8deb77
2016-10-24 15:52:21 -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
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
Volker E 0282b9648f Hovercards: Improve layout of settings dialog to design spec
Improving Hovercards' settings dialog to fulfill design specification.
Also fixing HTML structure by removing invalid `radiogroup` element and
adding missing `</div>` element, fixing related QUnit test.

Changes:
 * Add close icon instead of using text
 * Style header as a table for alignment
 * Remove redundant "OK" button and have it replace "Save" instead
 * Update text of "OK" button to "Done"
 * Fix description for translation of "Done" button
 * Fix qunit and selenium tests
 * Remove unnecessary markup and less
 * Add mediawiki-ui-button and mediawiki-ui-icon dependencies
 * Shrink dialog width some per design spec
 * Fix dialog horizontal position calculation to remove hard-coded value

Bug: T138612
Change-Id: I7395e3438836149becdd576942bdaf6f21b4163f
2016-08-10 14:01:36 -04:00
jdlrobson 7975fde745 Remove the need for global mw.popups.triggers
Change-Id: Iec1dd93a83fe393bd8717884ab4bab692ad7b6f4
2016-06-06 09:30:54 -04:00
jdlrobson dedb61caf9 Drop support for non-SVG browsers
According to caniuse.com SVG support is available
from IE > 8, Firefox > 3, Safari > 3.1 and Android
> 2.3. Android 3-4.3 does not support masking.

Out of all these browsers, considering market share
and ResourceLoader support, none of these browsers
are of concern to us. In IE8 for example we do not
run JavaScript for our end users. Thus we should remove
this fallback support.

Changes
* Remove createImgThumbnail method and its test
* Groups duplicate CSS groups
* Refactor createThumbnail function
** Leave a FIXME on some curious code

Change-Id: I59ac2e320b2e07815bc4136d5942016fdc1d4340
Bug: T135554
2016-05-31 15:23:07 -07:00
jdlrobson 765aa40cc1 Replace use of jStorage with mw.storage
Use the standardised MediaWiki storage system for the simple use
case of disabling/enabling Popups. The jStorage library is 12kb
uncompressed and cannot be justified for this usage (and if it becomes
the standardised library mw.storage will begin using it)

This means that browsers with full localStorage or no localStorage
support will not be able to disable Popups. Given the current ecosystem
of the web - localStorage is widely supported - and the fact that grade
A browsers enjoy localStorage support - this is not a problem.
See https://github.com/wikimedia/mediawiki/blob/REL1_27/resources/src/startup.js#L59

Changes:
* Stop using jStorage
* Cleanup and migrate previous values in jStorage with plan for removing this
in a month.

Bug: T136241
Change-Id: I6dac2911e84d6cb20731be34add01576cf407c46
2016-05-30 12:30:38 -07:00
Baha aba43fd560 Switch to Schema:Popups revid 15597282
Bug: T131315
Change-Id: I2ed18e00afb3e355327b417e68e5930b46d49086
2016-05-24 14:26:46 -07:00
Sam Smith c4667fd133 Convert isUserInCondition from async to sync
As there are an unmanageable amount of synchronous checks of
mw.popups.enabled, convert mw.popups.experiment.isUserInCondition to a
synchronous method.

Follow on I4959749.

Bug: T132604
Change-Id: Ide07e62868c77bfcd78af58dcec7303a35a72157
2016-05-19 14:06:17 -07:00