Commit graph

346 commits

Author SHA1 Message Date
Sam Smith 8b311aa159 Hygiene: FETCH_END -> FETCH_COMPLETE
Ideally, the preview model is resolved after 500 ms, regardless of
whether the internal gateway takes 100 or 300 ms. Given this, there's an
important distinction to be made between the "fetch" ending and it
completing and their associated actions.

Changes:
* Dispatch the FETCH_COMPLETE action when the preview model is resolved.
* Update the reducers accordingly.

Change-Id: I62c9cb0430284b76338ea80bd170cac5af4be9d0
2017-03-30 17:47:13 -07:00
Sam Smith da7325a169 changeListeners: Conditionally empty link titles
If the user has disabled PP via the settings dialog or they aren't in
the experimental condition, then link titles shouldn't be emptied.

Because this behavior has to respond to the user enabling/disabling PP
within the same page session, change the linkTitle change listener
rather than conditionally registering it.

Bug: T161277
Change-Id: I53c1a1d3e4436e2ffe08da27da388f394f4e8817
2017-03-30 17:39:39 -07:00
Sam Smith 82648226ef renderer: Bind behavior when preview is shown
Binding the behavior to the preview before it's shown means that the
application will respond to user interactions with the preview even
though it's transparent.

This fixes scenario 4 from T159490.

Bug: T159490
Change-Id: Ia2d06869868d07af60bdeb49d46612a4a0dc02e9
2017-03-30 17:37:58 -07:00
Sam Smith ae9733b2f0 eventLogging: Log abandon event when user dwells
If the user abandons link A (or preview A) and immediately dwells on
link B, then log a "dismissed" or "dwelledButAbandoned" event.

In this context, "immediately" means before the ABANDON_END action is
dispatched, which, currently, is 300 ms after the ABANDON_START action
is dispatched.

Bug: T159490
Change-Id: I49f0f5dfb3e6c08844f1794fee8cb6170e93981b
2017-03-30 17:15:26 -07:00
Sam Smith 90d54eca64 eventLogging: Extract createAbandonEvent function
Reducer changes:
* Add tests for ABANDON_END case.
* Extract the body of the ABANDON_END case into the createAbandonEvent
  helper function.

Additional changes:
* totalInteractionProperty -> totalInteractionTime elsewhere in the same
  file.

Bug: T159490
Change-Id: Ifff34271395f330b83cfe487e84800fe2d6f3811
2017-03-30 17:14:52 -07:00
Sam Smith d6cc8fa7cb eventLogging: SETTINGS_SHOW logs an event
Reducer changes:
* Make the eventLogging reducer queue a "tapped settings cog" event when
  reducing the SETTINGS_SHOW action.

This was discovered while testing I6ce7d72b.

Bug: T159490
Change-Id: I6ce7d72b364d20c71b0e2cfed98e99f7997895e5
2017-03-30 17:14:08 -07:00
Sam Smith 29963edb09 preview: Add click behavior
Like dwelling and abandoning, clicking on a preview is the same as
clicking on a link.

This fixes scenario 3 from T159490.

Bug: T159490
Change-Id: I6d9ff52b62bec93ebfcc9b6d267a46cf961852fb
2017-03-30 17:08:08 -07:00
Sam Smith df7868ea3f eventLogging: Model interactions in EL reducer
For now, mirror the interaction modelling in the preview reducer in the
eventLogging reducer to handle the user either:

* Repeatedly dwelling on and abandoning a link.
* (Repeatedly) moving their mouse between the link and the preview.

This fixes scenarios 1, 2, 5, and the general issue from T159490.

Bug: T159490
Change-Id: Ia771f325e541c107348b16b47c5b786c97847652
2017-03-30 17:03:06 -07:00
Sam Smith 59ea7a3162 actions: Increase API request delay to 150 ms
Step 1 of T161284. Given that the median API response time (as measured
by the client) is ~115 ms [0] and the API response is artificially
delayed so that the preview starts fading in at 500 ms, we can increase
the API request delay to 150 ms without affecting the current UX while
decreasing the number of incidental HTTP requests triggered by the user
glancing their mouse over a link to another page.

[0] https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews

Bug: T161284
Change-Id: I4c4a766467cdb4cd47c4231c1106c35bab67855e
2017-03-29 09:42:38 -07:00
jdlrobson b01e11c1f9 Popups doesn't need to depend on EventLogging
When EventLogging is unavailable do not initialise the EL-related code
or try to send any events.

When EL is enabled for a brand new user we request an additional module
during boot causing an additional HTTP request. Page Previews continues
to boot normally regardless of whether the request fails.

This approach doesn't impact boot or first paint time. Once the module
is loaded once it should be cached locally, subject to the
ResourceLoader's policy. Moreover, the RL will not attempt to load the
module twice so this doesn't impact the performance of other modules.

Bug: T158999
Change-Id: I7ed7f00d52279151ece23e5aced4f2adb0f7fdc3
2017-03-23 18:26:12 +00:00
Piotr Miazga aa81d6aaad Show Page Preview on mouseenter and keyup events
Changes:
 - remove focus events listeners as they are triggered after switching tabs
 - show PagePreview on keyup event

Bug: T158631
Change-Id: I7533f896604e0e0a8ea6e900ae4f7d12b6458836
2017-03-16 21:16:44 +01:00
Sam Smith a5e1cab732 Hygiene: Remove createRootReducer
Redux.combineReducers is equally self-documenting.

Change-Id: I726fbc782fbb59aad0c8dd3c8eb168a302415e6c
2017-03-15 13:23:23 +01:00
Sam Smith c5675842a3 Hygiene: Remove createBoundActions
Redux.bindActionCreators is equally self-documenting.

Change-Id: Ib10023b4d3a4d6dae0a7aca40f04c3f37abf05dd
2017-03-15 13:23:11 +01:00
Sam Smith c10c3c3771 renderer: Remove attributes don't set them to ''
IE doesn't appear to update/redraw the SVG image element when Setting
its clip-path attribute to '', not removing it. This is problematic as
the attribute is always set to a default value (in the createThumbnail
function) before the preview is laid out.

Bug: T160237
Change-Id: I4559ff5018b8f4ecf06f6f5d9462a999d9726b94
2017-03-14 08:46:05 -04:00
Baha 9a94300858 Log events to statsv for monitoring PagePreviews performance
For logging to work:
1. $wgWMEStatsdBaseUri needs to point to a valid statsv endpoint,
   e.g. 'https://en.wikipedia.org/beacon/statsv'.
2. $wgPopupsStatsvSamplingRate needs to be set. Note that the codebase
   already contains the EventLogging functionality, which is configured
   separately. Separately configuring different logging mechanisms
   allows us to avoid sampling mistakes that may arise while choosing
   one or the other. For example, let's say we want to use EventLogging for
   10% of users and statsv for 5%. We'd sample all users into two
   buckets: 50/50. And then we'd have to set the sampling rates as
   20% and 10% respectively, only because of the bucketing above. To avoid
   this kind of complications, separate sampling rates are used for each
   logging mechanism. This, of course, may result in situations where a
   session is logged via both EventLogging and statsv.
3. The WikimediaEvents extension needs to be installed. The extension
   adds the `ext.wikimediaEvents` module to the output page. The
   logging functionality is delegated to this module.

Notable changes:
* The FETCH_START and FETCH_END actions are converted to a timed action.
* The experiments stub used in tests has been extracted to the stubs
  file.

Logged data is visualized at
https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews

Bug: T157111
Change-Id: If3f1a06f1f623e8e625b6c30a48b7f5aa9de24db
2017-03-14 08:51:10 +00:00
Sam Smith 568b7a09a1 rest: Always scale thumbnail's largest dimension
... thumbnails.

A good example of the difference in behaviour of the PageImages API is
here <T156800#3087507>. The API defers to File#transform, which scales
the largest dimension of an image, not always the width, e.g. if an
image is 1000px x 2000px and the request is for a thumbnail "of 1800px",
then the thumbnail will be 900px x 1800px.

Bug: T156800
Change-Id: I64bc2244ee78a594298d8637233b0da1083700eb
2017-03-10 10:44:37 +00:00
Piotr Miazga 5328cf4681 Hygiene: Move EXTRACT_LENGTH to constants
Keep all configuration-like values in one file.
Changes:
 - moved EXTRACT_LENGTH to constants.js file

Change-Id: Ibe5ecfc60f2c09a30a9ecb3586bc5fb6a7365476
2017-03-09 22:08:10 +00:00
joakin eff14acbba Tooling: Use latest stable webpack
Webpack 1.14.0 is an old version, switch to using latest stable which
has better documentation, tree shaking, ES2015 modules and a core team
of contributors with funding. See https://webpack.js.org/

Additional changes:
* Recompile the frontend assets

Change-Id: I2c5940276e99dee104d04c6a0b83d8ab36a99df5
2017-03-08 19:27:40 +01:00
Sam Smith d4caff9774 rest: Don't scale unscalable thumbnails
If the image isn't an SVG then it shouldn't be scaled past its original
dimensions. Handle the case where the requested thumbnail can't be
generated on the server as the original is too small ( < 320px,
currently [0]) in the same way.

Moreover, if the image happens to have the exact dimensions that we're
requesting (300px or 600px wide, currently [1]), then use the original
image to avoid unnecessary work/pressure on caches.

Supporting changes:
* Update the SVG_RESTBASE_RESPONSE fixture to use the extension returned
  by RESTBase (and the PageImages extension) for the thumbnail:
  .svg.png.

[0] https://github.com/wikimedia/restbase/blob/master/v1/summary.yaml#L121
[1] https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/constants.js#L2

Bug: T156800
Change-Id: I5d0aa161e80869e4b4f5425d906d1e753047a3a3
2017-03-08 18:44:23 +01:00
joakin 1b81d0fdfa Bundle dist versions of redux and redux-thunk
Instead of importing the modules from sources (which you should do if
you properly define NODE_ENV and use uglifyjs from webpack) import the
already compiled files on the npm packages.

This results in 5kb less on the compiled bundle.

Change-Id: I83732ea79a59b611c117ddcf4c262948c795f642
2017-03-08 12:41:03 +01:00
joakin 49d4396e7a Hygiene: Remove global variable mw.popups in most places
Now that most unit tests are run in node with common.js for loading
sources there is no reason to keep global variables around exposing all
the sources.

Only exception is the only qunit integration test processLinks.test.js
which still consumes mw.popups.processLinks, which is the only global
variable remaining in the codebase.

Changes:
* Remove references to mw.popups in code comments and reference the JS
  file instead
* Remove popups.js which exposes all common.js modules as global
  variables
* Export mw.popups.processLinks in processLinks.js for testing in
  processLinks.test.js

Change-Id: I91066654b9282f73a80eb1ba5018bd091656c61d
2017-03-06 17:10:35 +01:00
Baha b40a24c15c Allow showing non-free images when using MediaWiki API
We used to query the MediaWiki API to only return non-free images.
This patch allows us to query the API for images with any license.
The RESTBase end point is already returning images with any license.

Bug: T158632
Change-Id: I9ac60b6f74a7f7eb2cb160ee522c2c3a26dd0858
2017-03-06 11:33:17 +00:00
Derk-Jan Hartman b61c1ef993 Correct 1px offset error in SVG mask.
All the other masks use an 8px offset for the triangle. But this
specific mask used 250 - 243 = 7px offset.

Bug: T153840
Change-Id: Ib72842d18bd844ff37509cf5bf1dedd4e0f99dbc
2017-03-03 11:38:40 -05:00
Sam Smith 720cfbdcd7 restbase: Use thumbnail when generating thumbnail
Rather than manipulating the URL of the original image to get the URL of
the appropriately sized thumbnail, manipulate the URL of the thumbnail
image.

While we could manipulate either the thumbnail or original image URL,
there are subtle differences between the two, so manipulating the latter
makes the generateThumbnailData function as simple as possible, e.g. we
don't have to splice in "/thumb" after "/commons".

Also, ensure that the thumbnail's dimensions are scaled appropriately.

Bug: T156800
Change-Id: I6825bad14b1131dc81f23dcf120cf8ffb7d7b4f6
2017-03-01 15:41:50 +00:00
Baha 5d4cc8d15a Allow bucketing anons
* Logged in users bypass bucketing. They keep working as before.
* When Page Previews is configured as a beta feature, logged out users
  won't see the feature.
* If an anonymous user has enabled/disabled the feature using
  the settings cog then they will see or not see the feature
  depending on the value of their setting.
* The other anonymous users are bucketed. By default 90% of these
  users see the feature, the other 10% don't. These numbers can be
  controlled by the config variable `wgPopupsAnonsEnabledSamplingRate`.

Bug: T157700
Change-Id: I5307587b10f4849c4e82d3b064ff759121c2de67
2017-03-01 10:45:32 +00:00
Sam Smith f54f92402c storage: Fix UserSettings#hasIsEnabled
mw.storage#get doesn't take a default value to return if the underlying
storage is disabled or the key is missing. In the former case it'll
return false and in the latter it'll return null, i.e. in neither case
will it return undefined.

Bug: T157700
Change-Id: I3f653c11468e17b64765e85ebb3b8f03e311352a
2017-03-01 10:45:32 +00:00
joakin 82e315b124 Tests: Migrate {integration,actions}.test.js to node qunit
Because of the globals mw.popups.wait usage and mocking in both actions
and integration, they need to be migrated in a single step, fixing them
both to require wait.js and mock using mock-require instead of the
global variable.

Additional changes:
* Fix FIXMEs about actions.js using the global mw.popups.wait instead of
  the require one.
* Fix the unit tests to use require mocking for wait.js instead of
  global variable mocking in both integration and actions tests
* Change tests that use deferreds and promises to be async qunit tests
  (Deferreds are asynchronous with jQuery in node, apparently they
  weren't in the browser)
* Change integration.test.js to use require on Redux and ReduxThunk

Change-Id: I8e3e87b158bd11c9620e77d0a73e611cf9e82183
2017-02-27 18:17:28 +01:00
Sam Smith 938a4b85d4 Hygiene: Remove checkin instrumentation
The "checkin" part of the Popups schema was superseded by the
ReadingDepth schema, the implementation of which is tracked by T155639.

As well as removing all checkin-related code, update the Popups schema
to the latest version - the version that doesn't have the checkin
property.

Bug: T155639
Depends-On: I762ec3fc91decf3cffa869dbd783faf62f01329a
Change-Id: If764917b6e121e1f9db980a4efa30c0f7a166197
2017-02-27 14:48:47 +00:00
joakin 2ca5fed3ee Tests: Migrate changeListeners/render.test.js to node-qunit
The render change listener is hard coupled to the renderer file, so in
order to migrate the test, instead of stubbing a global variable, we had
to either inject the renderer into the change listener factory as done
everywhere else, or mock the require call.

In order to do one thing per commit, we're mocking the require call
here to get the migration done, but added a FIXME to use dependency
injection instead in a future change.

Change-Id: I50f82cdc9664d34b8a8ccc1ff368f7209404159d
2017-02-22 12:14:05 +01:00
Piotr Miazga 6a1948d729 Hygiene: Use CommonJS in gateway/rest.js
Changes:
 - removed unnecessary IIFE closure

Change-Id: Ic91c29d9d5e09573145c8667a6a941233b91b3fd
2017-02-21 17:57:08 +00:00
joakin 78cb95cda2 Hygiene: Remove unnecessary IIFE and use proper requires
In index.js. Instead of using the global variable/object popups, require
things from their files so that we can remove the global variables when
we can run qunit tests with commonjs in node.

Change-Id: I85408f01eca27f97cf46b2076176fcc16c037829
2017-02-20 18:39:51 +01:00
joakin f1e6e2bfa1 Hygiene: Remove unnecessary IIFE in gateway/mediawiki.js
Change-Id: If6d76c2915b13f9871dea3b7f33cb74ec5906566
2017-02-20 18:39:49 +01:00
joakin 0260325bb9 Hygiene: Remove unnecessary IIFE in schema.js
Change-Id: Ic152668ff7e4c96fb63934283897a51e81e58dd4
2017-02-20 18:39:47 +01:00
joakin 5fc46b4cdd Hygiene: Remove unnecessary IIFE in renderer.js
Change-Id: I89a1ac2205385db8e2f2c040ac22d2f4de793a18
2017-02-20 18:39:45 +01:00
joakin 2c1a30e35e Hygiene: Remove unnecessary IIFE in processLinks.js
Change-Id: Icd6ab0a9b0a189c19310915cb77f861459aecddf
2017-02-20 18:39:42 +01:00
joakin d54cfc2e42 Hygiene: Remove unnecessary IIFE in previewBehavior.js
Change-Id: I4f6f4094ba545f827dad28e966d69e6ecec3cea2
2017-02-20 18:39:33 +01:00
joakin f2e9654d23 Hygiene: Remove unnecessary IIFE in checkin.js
Change-Id: I84afc09540f24af425e2abd4603115bdf0030aa7
2017-02-17 06:39:25 +01:00
joakin 27d1ad8318 Hygiene: Remove unnecessary IIFE in changeListeners/render.js
Change-Id: Ib313ddd52d59c6422e02d98b67b5b475b35e79b7
2017-02-17 06:39:21 +01:00
joakin dfe44acef2 Hygiene: Remove unnecessary IIFE in changeListeners/linkTitle.js
Change-Id: Ifc882875475d1bd5056879c59314030a6f616daa
2017-02-17 06:39:16 +01:00
joakin 36287477d0 Hygiene: Remove unnecessary IIFE in changeListeners/footerLink.js
Change-Id: Id1535ce6af0d2dda0e58b784180d3d072844daa8
2017-02-17 06:39:10 +01:00
joakin dfb5ed6688 Hygiene: Remove unnecessary IIFE in changeListeners/eventLogging.js
Change-Id: If379092ab5132b47525ba350d6bbb4cee4568a15
2017-02-17 06:39:00 +01:00
joakin 8d22cf248a Hygiene: Remove useless IIFE in settingsDialog.js
Change-Id: I2885e02d0e61f9e98e086ce4399bc9a18517e531
2017-02-17 06:38:55 +01:00
joakin fd91fe7d93 Hygiene: Remove unnecessary IIFE in wait.js
Change-Id: Ia360bbd99b3d657116c0ba11c5acca726e4602a4
2017-02-17 06:38:46 +01:00
joakin dc7b05c27e Hygiene: Remove unnecessary IIFE in actions.js
Change-Id: I6031efbce04d4cd91a37690469d49ffd0372b41c
2017-02-17 06:38:35 +01:00
jdlrobson 6d859a73a3 Resize thumbnails images returned by REST endpoint
This change resizes thumbnails to the appropriate width
based on the value of mw.popups.gateway.THUMBNAIL_SIZE

Tests cover
* When requested thumbnail is < than original size
* When requested thumbnail is > than original size
* When requested thumbnail is an svg and originalimage
smaller than requested thumb size

Bug: T156800
Change-Id: Ib375b97e2bc959e91de5177efc3df1f2ded54a5b
2017-02-16 16:19:14 -08:00
joakin ba4844cdc2 Tooling: Separate built resources from RL resources
In order to automatically verify in CI that the built assets are up to
date with the commited sources, we need to keep the built assets in
a folder separate from the RL assets.

* Rename the compiled assets folder to industry standard `dist`

Change-Id: I8c5898f9bb29fee7164a7038b835a5f7efd33dbc
2017-02-14 18:39:38 +01:00
Renamed from resources/ext.popups/index.js (Browse further)