Commit graph

937 commits

Author SHA1 Message Date
Volker E df71836742 Align colors to WikimediaUI Style Guide and palette
Setting text color to `#222` and border color to `#a2a9b1` as
everywhere else.
Also making use of more recent LESS functionality with multiple
arguments per mixin to remove unecessary duplication of code and
change static values to central LESS variables where applicable.

Change-Id: I394c7e7e1369ff38b7ea91c7faebe773bcb2948d
2017-10-08 07:49:39 -07:00
smarita 3ec185cb01 Consider using more common image sizes for Page previews
Modified necessary files to increase size of popup from 300px to 320px

Bug: T173434
Change-Id: I47bbe9defe961008163551d5be4fc7b1ca08d0d1
2017-10-01 22:02:29 +05:30
Volker E 0c64c25c29 Unify box-sizing to common LESS mixin
Change-Id: Ifd3e7dd9be68c09b362c0fb8071cd9f27f846505
2017-09-21 15:32:09 -07:00
joakin 84e30f4613 Revert usage of Promise A+ in actions.js#fetch
.catch is only available with jQuery 3, which is only now starting to
roll out (see https://phabricator.wikimedia.org/T124742)

This patch rolls back the usage of .catch introduced in
https://gerrit.wikimedia.org/r/#/c/373327/ to use .fail, but only the
source part, given that patch introduced unit tests for this behavior

Bug: T174724
Change-Id: I6e1c342d812b35846061266bc59e493e87423fd8
2017-09-01 12:10:50 +02:00
Piotr Miazga f8498ce70c Store map files under .json extension
We cannot serve .map files from production servers. This makes
Popups extension difficult to debug. As a simplest solution we
decided to store map files as .json files so we can easily access
js maps files.

Changes:
 - changed webpack config to store map files under .json extension

Bug: T173491
Change-Id: Iaa55f75a8c5f3e8f1f169b3ac33241cc54f0413f
2017-08-29 21:18:35 +02:00
Piotr Miazga cb72bf4e82 Do not use keyword const as it's part of ES6 syntax
Changes:
  - instead of const use var

Bug: T174424
Change-Id: I3d786614b5acbfe9a05c332edd3a14c2e5afa417
2017-08-29 16:37:46 +02:00
Bartosz Dziewoński b63d2262f8 Don't use ES6 Number.isNaN
Number.isNaN is a new function introduced in ECMAScript 6.
MediaWiki only requires ECMAScript 5 supports from browsers.
Notably, Opera 12 does not have Number.isNaN. Instead, use
the global isNaN function (which behaves the same except for
non-numeric inputs).

Change-Id: If436cd26b21ce0336dfbc37144f6226e7b948e5e
2017-08-28 21:22:07 +02:00
joakin e865409593 Hygiene: Don't rely on .fail, use Promises/A+
Instead of using the non-standard old Promise implementation by relying
on .fail, migrate it to .catch.

In this specific case, where we were chaining promises with $.when, the
semantics from going from fail to catch actually change. .fail keeps the
promise in a rejected state, while .catch will change it to resolved
unless another error is thrown.

As such, when changing it to .catch, the catched error will be re-thrown
to keep the promise in a rejected state, so that $.when.then is not
executed.

Additional changes:
* Make actions.js#fetch return its promise for ease of testing.
* Test FETCH_FAILED, which was fully untested.

Bug: T173819
Change-Id: Ibd380b714586979c6e60b4c969d17f36a0796b52
2017-08-23 19:36:05 +02:00
jdlrobson e4e9bb3bd6 Popups A/B test infrastructure
Introduce PopupsAnonsExperimentalGroupSize config variable. This defines
a population size who will be subject to experimentation. If the group
size is undefined or 0 (default) and PopupsBetaFeature is false
(default value) Popups will be enabled for everyone. If it is any other
value, half that group will see page previews.

Drop the config variable PopupsSchemaSamplingRate - we will now only
EventLog when an experiment is occuring. This means we can simplify the
MWEventLogger class as shouldLog will always be truthy. Given server
side eventlogging is only used for preference changes
traffic should be low and not need sampling.

Introduce getUserBucket which determines whether a user is in a bucket
on, off or control based on the value of
PopupsAnonsExperimentalGroupSize. Add tests showing how these
buckets are calculated.

Caution:
A kill switch wgPopupsEventLogging is provided for safety.
It defaults to false. Before merging, please check if any config changes
are necessary.

Bug: T171853
Change-Id: If2a0c5fceae78262c44cb522af38a925cc5919d3
2017-08-17 21:07:07 +00:00
Piotr Miazga 910dd0408f if preview count stored in localStorage is not a number override it
There are some cases when the preview count stored in local storage
evaluates to NaN. When this happens we should override the value
to zero, store it in localStorage, and return it.

Bug: T168371
Change-Id: Ic44b7c7b5b716f6a0859f33278d56d2d95bbfb3e
2017-08-17 18:29:56 +02:00
Baha 3ac3769aa7 Remove event logging duplication detection and logging
We haven't seen the PP EventLogging instrumentation produce duplicate events
for weeks.

Bug: T172106
Change-Id: I6f3d7c0cdbf23161f63259e4d20d8a710376468b
2017-08-16 11:46:35 -04:00
Piotr Miazga d07441ec7f getPreviewCountBucket should return unknown when no bucket is found
Under some unknown circumstances getPreviewCountBucket() is called
with a value that is not a -1 or a natural number. When that happens
function returns 'undefined bucket' which causes eventLogging to
fail. I wasn't able to reproduce the issue, it might be specific
to browser/os. The safest way is to return 'unknown' for any other
case.

Bug: T168371
Change-Id: I374bb629762a86ac06a18e775d3c1a14682c9f55
2017-08-15 16:12:26 +02:00
joakin f37b76f8b4 Hygiene: make integrations/mwpopups pure
Instead of registering global variables in a function, make it pure
return the external interface and set it to mw.popups in the
src/index.js entry point.

Explicitly comment on index.js what is being set and why.

Bug: T171287
Change-Id: I94d467bfa7fa6e56033dd254518ad50b5dde5bfc
2017-08-09 16:07:08 +02:00
Piotr Miazga 8510b4b942 Allow 3rd party to check Popups enabled state by accessing mw.popups object
Changes:
 - introduced js module defining mw.popups object
 - introduced isEnabled() method which checks the redux store to retrieve
 isEnabled status

Bug: T171287
Change-Id: I523369831e2aa8a915ed1cb001b35d13b770f9da
2017-08-08 15:38:01 +02:00
joakin e6081106f1 Use EcmaScript modules instead of common.js modules
Why: Because they are the approved standard by TC39 and Ecma for
JavaScript modules.

Changes:
  * Wrap mw-node-qunit in run.js to register babel to transpile modules
    for node v6
  * Change all sources in src/ to use ES modules
    * Change constants.js to be able to run without
      jQuery.bracketedDevicePixelRatio given ES modules are hoisted to
      the top by spec so we can't patch globals before importing it
  * Change all tests in tests/node-qunit/ to use ES modules
  * Drop usage of mock-require given ES modules are easy to stub with
    sinon

Additional changes:
  * Rename tests/node-qunit/renderer.js to renderer.test.js to follow
    the convention of all the other files
  * Make npm run test:node run only .test.js test files so that it
    doesn't run the stubs.js or run.js file.

Bug: T171951
Change-Id: I17a0b76041d5e2fd18e2d54950d9d7c0db99a941
2017-07-31 23:05:44 +00:00
Volker E eebf17ac3f Remove obsolete vendor prefixed properties and at-rules
Opera 12.10 supported unprefixed animations, gradients, transforms &
transitions. See http://www.opera.com/docs/changelogs/unified/1210/
Removing support for Opera 12.0x versions.

Change-Id: I3476db173433c430f654e12ea1f17d2721410b83
Depends-on: Ie8edbcd7f85c713ea2156706ea3a3a7b423d8d9d
2017-07-31 21:39:42 +02:00
joakin 02507fb74d Hygiene: Move settingsDialog UI code to ui/
Bug: T171951
Change-Id: I58f77737456e1f4b9db6631f83e4b0f14212c939
2017-07-31 19:14:18 +00:00
joakin 31fa60d32c Hygiene: Move ui renderer.js to ui/ folder
Seems appropiate to group the UI portions of the source under a ui
folder.

Bug: T171951
Change-Id: I6d4317abea4e2a8e273e13fc40a7445bb54628ef
2017-07-31 19:11:41 +00:00
Sam Smith ba3c0b7f76 Hygiene: i13n: Return false over not sampling
Previously, if the browser didn't support the Beacon API, then
instrumentation/eventLogging#isEnabled would bucket the user with a
sampling rate of 0, which is equivalent to returning false. This change
simply does the latter.

Additional changes:
* Update the documented module names of the instrumentation/eventLogging
  and statsv modules.

Bug: T168847
Change-Id: I7ae5c10da42ca614b5b1a6619f9555e5665344cf
2017-07-26 17:06:58 +00:00
Sam Smith 8e00ecc5d1 i13n: popupEnabled = false for disabled event
... for consistency with the server-sent disabled event introduced in
I63faecb0.

Bug: T167365
Change-Id: I3a96df5279f6f0f4e573765735ab0e1fc6f406a8
2017-07-24 17:31:59 +00:00
jenkins-bot 59290bcf4a Merge "Remove duplicate events filtering" 2017-07-06 18:05:18 +00:00
jenkins-bot eebd0c5e8b Merge "Send disabled event from settings windows" 2017-07-06 18:05:17 +00:00
Piotr Miazga 426356e822 Remove duplicate events filtering
We had instrumentation for over 4 weeks and duplicate events rate
was very low. We want to keep stats so we check the duplicate events
rate but there is no need to filter those.

Bug: T167365
Change-Id: I72585beb21e9db589e45eeace657ef25f432abc9
2017-07-06 17:38:42 +02:00
Piotr Miazga 7e2c79ae0d Send disabled event from settings windows
Changes:
 - introduced new event 'disabled', sent from settings popup
 - added unit tests for 'disabled' event handling

Bug: T167365
Change-Id: I048b38122b8843199c86fd1ed9ec2ff21767e114
2017-07-04 19:08:37 +02:00
jenkins-bot 6a1176c0d2 Merge "Enforce no top&bottom margins on lists on page previews" 2017-07-03 13:52:28 +00:00
Piotr Miazga a82e54bf2d Override eventLogging to enabled when debug flag is on
In order to debug the EventLogging instrumentation in production
environments, we want to be able to bucket ourselves at will.
When the debug flag (?debug=true) is passed send all events
for given page view.

Bug: T168847
Change-Id: Id1b13b0ecaa791b4f26be4d1151bdbbe5270b64d
2017-06-30 18:52:51 +00:00
Piotr Miazga 06ef34c05f Enforce no top&bottom margins on lists on page previews
Changes:
 - set margin-top and margin-bottom to 0 on following elements:
   ul, ol, li, dl, dd and dd

Bug: T168941
Change-Id: I80478de046d7944fde3c0de3f96f5c9dc4623c36
2017-06-30 20:51:21 +02:00
Piotr Miazga 450e6bc34c Allow events without linkInteractionToken to be logged
Changes:
 - when event doesn't have linkInteractionToken do not check for
   duplicated tokens
 - hygiene, move event duplication logic into separate functions
   for better readability

Bug: T168449
Change-Id: I3ae197567ec9f67e104af109d4f1a1c1a6769d32
2017-06-30 18:03:40 +02:00
Baha def31b9e90 Remove paragraph margins from HTML extracts
This allows us to display an exact number of lines of texts in a preview.

Bug: T168332
Change-Id: I2066fbca2fce77c9eb1f0fe4278b96f13383e9a6
2017-06-20 16:11:22 -04:00
Sam Smith dcf8532cdf Hygiene: Group instrumentation modules
Following on from I4f653bba, since the schema and statsvInstrumentation
modules are similar, let's group/rename them:

  schema -> instrumentation/eventLogging
  statsvInstrumentation -> instrumentation/statsv

Change-Id: Ic59e0da7d4917f6733fd090f15d3c269af863f05
2017-06-20 11:41:37 +01:00
Sam Smith 67eb3b1dcf i13n: Log EL events with mw.track
Currently, the mw.eventLog.Schema class samples per pageview. However,
we expect that if a user is bucketed for a session, then all
EventLogging events logged during that session are in the sample.

Moreover, loading the class in the way that we did - asynchronously,
using mw.loader#using - introduced an issue where the eventLogging
change listener would subscribe in the next tick of the JavaScript VM's
event loop and miss the "pageLoaded" event being queued (see T167273).

Changes:
* Make the schema module follow the form of the statsvInstrumentation
  module, i.e. make it expose the #isEnabled method, and add the
  associated getEventLoggingTracker function.
* Update the eventLogging change listener accept the tracker returned by
  getEventLoggingTracker.
* Update/fix related JSDoc documentation.

Bug: T167236
Bug: T167273
Change-Id: I4f653bbaf1bbc2c2f70327e338080e17cd3443d4
2017-06-17 00:51:32 +00:00
joakin 98d9415361 Hygiene: Rename builder vars on require preview/model
preview/model is just a module/namespace object, not a builder class or
similar.

This patch changes a couple of imports to reference some of the exposed
functions of the preview/model where required.

In a shiny future, this pattern would be:

    const { createModel, createNullModel } =
      require( '../preview/model' )

Bug: T165018
Change-Id: If6ad4611538ca4f24e2443c0c3ed433275e995a6
2017-06-16 14:50:19 +02:00
joakin 010a4d91a6 Hygiene: Simplify gateways
gateway/*/rest were copies of gateway/restProvider just passing
a different provider. Docs were the same, they were untested, and
looking at them they seemed like unnecessary abstraction.

This patch removes the plain vs html structure, and separates gateways
like before, by endpoint.

There is a light utility in gateway/restFormatters.js that adapts the
call from the rest gateway to use formatters.js functions. It needs
testing, that I'll add in the next patch.

The flow for creating a gateway ends up as follows:

1. index.js calls gateway/index#createGateway( mw.config )
2. createGateway chooses based on wgPopupsGateway and invokes
  * mediawiki.js#createMediaWikiApiGateway or
  * rest.js#createRESTBaseGateway w/ restFormatters.js#parsePlainTextResponse or
  * rest.js#createRESTBaseGateway w/ restFormatters.js#parseHTMLResponse

Changes:
* Removed src/gateway/{plain,html}/rest.js
  * Extracted formatter functions to src/gateway/restFormatters.js
* src/gateway/plain/mediawiki.js -> src/gateway/mediawiki.js
         * tests/node-qunit/gateway/plain/mediawiki.test.js ->
           tests/node-qunit/gateway/mediawiki.test.js
* gateway/restProvider{,.test}.js -> gateway/rest{,.test}.js
* Change gateway/index.js#createGateway to properly call the rest
  gateways with the rest formatters

Bug: T165018
Change-Id: Ia75695dfc192aad5bc581a68882514bad6c29646
2017-06-16 14:49:59 +02:00
joakin b12599c871 Hygiene: Move createGateway to gateway/index.js
And add tests, given it is growing in complexity.

Additional changes:
* Interface ext.popups.Gateway -> Gateway in docs

Bug: T165018
Change-Id: I8a12333ad9d14d6a7fbde11afc42f607881e8ea3
2017-06-16 12:46:05 +02:00
joakin 490583bcb9 Hygiene: Capture jQuery at construction
and not from global scope all the time.

Bug: T165018
Change-Id: I90b55a65a7ca25c2998c811a98401feaeced165e
2017-06-15 20:10:08 +02:00
jenkins-bot a3b5bddefe Merge "i13n: Extract experiments module" 2017-06-14 20:09:02 +00:00
jenkins-bot 08fa97d8e0 Merge "Increase line number count of preview with tall image" 2017-06-14 18:44:17 +00:00
Sam Smith 6159af3151 i13n: Extract experiments module
... from the statsvInstrumentation module so that the bucketing logic
can be shared with other instrumentation modules.

Change-Id: I5732fa539a3911939fa85fa88c102fa8dcfa5613
2017-06-14 11:04:32 -07:00
Baha 686c1306ba Increase line number count of preview with tall image
Previews with tall image now have 9 lines of text as opposed to
the previous 7 lines.

Bug: T165978
Change-Id: Ib3d39f5164663f9c9ccaa3c0a974d524fc2f9463
2017-06-14 10:22:02 -04:00
Piotr Miazga f2fbef6ec7 Implement html/rest.js gateway which handles HTML Restbase responses
Refactor existing Restbase gateway and extract shared logic into
shared Restbase provider. Also introduced new createNullModel()
which defines an empty preview model.

Additionally improve naming in new gateways/formatter so function
names are more explicity.
 * Htmlize() became formatPlainTextExtract() as it should be used
   only with plain text extracts
 * removeEllipsis() became  removeTrailingEllipsis() as it removes
   only trailing ellipsis.
 * src/gateway/index.js defines gateways by configuration name stored
   in extension.json

Bug: T165018
Change-Id: Ibe54dddfc1080e94814d1562d41e85cb6b43bfc1
Depends-On: I4f42c61b155a37c5dd42bc40034583865abd5d7a
2017-06-13 20:19:05 +02:00
Timo Tijhof 4996886eb9 eventLogging: Use base 32 instead of 16 for fnv-encoded hash
Follows-up 79f3b318d0.

Number#toString supports up to Base 32.
Same collision behaviour but with a shorter string.
Typical length with Base 32: 7 (max: 11)
Typical length with Base 16: 9 (max: 14)

Change-Id: I91e91341cbecdec24549ace6a6300550f5b449ee
2017-06-12 21:43:05 +01:00
joakin 38b061a468 Return empty extract if string is blank after formatting
Bug: T167626
Change-Id: Icb89cbe447226850231c3d7d74e61e5f6a5db809
2017-06-12 12:59:31 +02:00
Sam Smith fb8d54c7ce actions/rest: Use DB-key version of title
This reduces the number of 301 Redirect responses when fetching previews
from RESTBase.

Bug: T167633
Change-Id: I830947ab79e72dcc023193412c8d5bcee986e23f
2017-06-12 11:22:55 +01:00
Piotr Miazga ef283c2509 Extract rendering/parsing mediawiki responses into separate class
Page Previews should be able to consume HTML response generated by
MediaWiki. First we need to move out plain text crunching from
renderer.js and model.js. Mediawiki and Restbase gateways will have
to parse/htmlize plaintext into nice HTML by themselves.

Bug: T165018
Change-Id: I5d7e9f610bb809aa9fb035a4a9f96e9e8796c9d8
2017-06-09 18:34:25 +02:00
joakin 002f4c8e0c Use delegated events in container
...instead of 1 event per link

Supporting changes:
* Delegate events on the container when booting up
  * Check eligibility of title on event triggered
  * Pass the title from the event handlers into the actions instead of
    storing it in the dom
* Add title.fromElement as sugar over isvalid(gettitle())
  * Not tested as it is sugar over the other 2 functions
* Fix action tests and integration tests

processLinks to be removed next

Bug: T165572
Change-Id: I4d9837706dc77ec64121ac94410c0d2da83692e4
2017-06-08 12:31:06 -07:00
joakin 356678ffcd Add title#isValid
Which checks if a title is an eligible one for showing previews

Bug: T165572
Change-Id: I5ade3fb84d400293d24de05e10119996e711b41e
2017-06-08 12:29:33 -07:00
Sam Smith 343506bdfb gateway: Fix Accept header sent by rest gateway
... and update the RESTBASE_PROFILE constant to the latest "stable"
profile for the endpoint.

Prior to this change the Accept header sent by the rest gateway was

  application/json; charset=utf-8profile="..."

This was discovered while responding to T166605.

Change-Id: I00f277e724c561634b26c9ab10bd35332c6dba91
2017-06-08 12:19:32 -07:00
joakin ef2f99ef65 Rename getTitle.js to title.js
In order to create a title#shouldShowPreview function next in the
module.

Bug: T165572
Change-Id: I9e59bb0f525d2698f882543ca0d4a1bde6b2d5d2
2017-06-08 12:10:14 +02:00
Sam Smith 98864a7ce3 eventLogging: Add missing properties to "tapped settings cog" event
When there's an interaction, then the "tapped settings cog" event should
have the same properties set as the other interaction-specific events.

This was discovered while QAing T164256.

Bug: T164256
Change-Id: I4749b52656203c7e0c42ae742556ee996eee322a
2017-06-07 10:13:23 +01:00
Baha 39672c5f49 Make vertical preview show 7 lines
Bug: T165978
Change-Id: I3474ab271679d290be58afca0d31c25b17b0442b
2017-06-06 15:43:21 +00:00
Baha f44218d2da Reduce height of horizontal gradient
Change padding to margin, and set the exact height of the container
to 7 lines.

Bug: T165974
Change-Id: Id4f97dddfb554ee2276b890a06030522076f8c88
2017-05-31 16:16:02 -04:00
jdlrobson 918a74b1e6 Docs: Don't register methods as globals in documentation
This ensures all files belong to a module.
When generating documentation no globals should be present

Bug: T158236
Change-Id: I134f38620fe46db11ba94dbede739f4336e0482c
2017-05-26 10:35:07 -07:00
jdlrobson e21a640002 Run doc generation on npm test
The benefit of this is if there are any problems with the
documentation they will not enter our codebase.

We do a similar thing in MobileFrontend

Bug: T158236
Change-Id: I30329dd868fe596c490f95354c3226c9cd4a2fc7
2017-05-26 10:26:35 -07:00
Sam Smith 30e616a224 doc: Document reducers/eventLogging module
Bug: T158236
Change-Id: I327f11ba804063a912c355477e9247a99e9b6e32
2017-05-26 09:02:47 +01:00
Sam Smith 66234e021e eventLogging: Add perceivedWait prop to all events
... and the previewType property as well.

Per the Popups schema [0], the "opened" action should have the
perceivedWait and previewType properties set.

Bug: T166323
Change-Id: I957d123434a6b750aff6f5279865321a08367382
2017-05-25 17:00:13 -04:00
Sam Smith b6bca56ad8 doc: Document userSettings module
Bug: T158236
Change-Id: Ia2365c7d6d1b9bc7438a3824656f1dde2a0170fb
2017-05-25 18:24:57 +01:00
Sam Smith 0670597434 doc: Document gateway modules
Additional changes:
* Fix the summary not showing up for the Container interface.

Bug: T158236
Change-Id: I7c5dee1b4525c2db28b89e57604d2d073620293d
2017-05-25 14:34:25 +01:00
Sam Smith e4f4041846 doc: Document statsvInstrumentation module
Bug: T158236
Change-Id: I138a1ef5305c0b49415d5f2a914ba45fd6aa869b
2017-05-24 10:45:49 +01:00
Sam Smith e9fb33af3a doc: Document isEnabled module
Bug: T158236
Change-Id: Ib80a5951e41765b982fecf4e2ac33767e4ef1738
2017-05-23 13:26:56 +01:00
Sam Smith 445318111b doc: Document counts module
Bug: T158236
Change-Id: I27106436e693289aa0c4140cef8e3d6210bcd8fb
2017-05-23 13:26:56 +01:00
Sam Smith 9350df7111 doc: Document schema module
Bug: T158236
Change-Id: I365fbfc1725d92003e902301252d53d4620c2128
2017-05-23 13:26:56 +01:00
Sam Smith af24fe958a doc: Document wait module
Bug: T158236
Change-Id: I5bdd3d7bb6fd0eb219b01ae113dc5bc8bdf12d0a
2017-05-23 13:26:56 +01:00
Sam Smith 5d9561bc91 doc: Document preview/model module
Changes:
* Assign exports to exports rather than reassigning module.exports so
  that JSDoc can guess inner members.
* Tidy up parameter types with JSDoc's "nullable" syntax.

Bug: T158236
Change-Id: I7261d1bb3924c9f14301490f54d7813dcffc959b
2017-05-23 13:26:56 +01:00
joakin 293d7ebe8d Clear interaction after an event for it is logged in EL
Given that interactions end up with an event logged, there shouldn't be
a reason to keep an interaction active after it's corresponding final
event has been logged. See Tbayer's state graph.

This patch removes the current interaction if an event with that token
is logged, effectively finalizing it and making it impossible for the
token to be reused from the state tree again.

Additional changes:
* Pass the logged event with the action EVENT_LOGGED so that the reducer
  can determine if it needs to do anything else.
* Since the interaction is removed, when undefined, guard against
  actions that use state.interaction freely. (Only allow BOOT,
  LINK_PREVIEW, and EVENT_LOGGED)

Bug: T161769
Bug: T163198
Change-Id: I99fd5716dc17da32929b6e8ae4aa164f9d84c387
2017-05-16 11:25:41 +02:00
Sam Smith cf0ea9db7b actions: Mix title and namespaceID into LINK_DWELL
This fixes a bug in I8a63d82, where the pageTitleHover and
namespaceIdHover properties of EventLogging events weren't being set.

Bug: T164256
Change-Id: Ie2c2d253f6508b89d48129fd17a902e5ded7cad5
2017-05-15 19:02:27 +01:00
Sam Smith 35bf613964 eventLogging: Add missing *Hover properties
Action changes:
* Include the namespace ID in LINK_DWELL.

Reducer changes:
* Add the createEvent helper, which adds the pageTitleHover and
  namespaceIdHover properties in the event.
* Create "dismissed", "dwelledButAbandoned", and "opened" events using
  the createEvent helper.

Additional changes:
* Store the target page's associated mw.Title using jQuery.
* Update the eventLogging reducer's test cases so that all LINK_DWELL
  representations include title and namespaceID attributes.
* Create the createStubTitle factory function, which returns a minimal
  usable mw.Title stub, and use it in the actions and integration test
  cases.

Bug: T164256
Change-Id: I8a63d82a65324680dff9176020a8ea97695428c4
2017-05-13 08:27:58 +01:00
joakin 7bd29bb058 Enable production settings for the production bundle
It enables certain optimizations, see:
* https://webpack.js.org/guides/production-build/

Code is minified by uglifyjs.

Aditional changes:
* Add banner so that sources are not minified by ResourceLoader

Bug: T160061
Change-Id: I50c9148ebf2d860db42a24225bc128bfcfe56927
2017-05-11 10:37:45 +02:00
Sam Smith c44fddf8cd eventLogging: Discard events with duplicate tokens
I6a38a261 made the eventLogging change listener count and discard
duplicate events and count duplicate tokens.

While we isolate the issue(s) that lead to duplication (reuse, likely)
of tokens, make the eventLogging change listener discard events with
duplicate tokens as well.

Bug: T161769
Bug: T163198
Change-Id: I0dbb16c37814d39d7aec35c8fb7cc7309704c550
2017-05-10 11:43:05 +01:00
Baha d6424cb59d QA: Test renderer#show
Binding the behavior has been left out as it requires some refactoring.
Ideally, that functionality should be tested via integration tests, which
is already done.

Bug: T133022
Change-Id: If2fd472847eb3557de97c7ec9619e8831e9bda6d
2017-05-09 17:15:01 -04:00
Baha 108f205e12 QA: Test renderer#layoutPreview
Bug: T133022
Change-Id: I7b7aa544b850e5487a621c522b28653db6056633
2017-05-09 17:13:16 -04:00
Sam Smith 2d3f5333b5 eventLogging: Round perceivedWait property
The perceivedWait property, introduced in I3fd253f1, must be an
integer.

Bug: T164256
Change-Id: If19e8694e5ac111af0f27146cf4ac03bd1a6ff82
2017-05-09 19:00:41 +01:00
Sam Smith 35cfc38b0b eventLogging: Add missing perceivedWait property
Per the Popups schema [0], the "dismissed" action expects the
perceivedWait property to be set.

Happily, the time until the preview was shown is already recorded and
used to determine whether or not to create a "dismissed" or
"dwelledButAbandoned" event.

Reducer changes:
* When creating the "dismissed" event, set the perceivedWait property to
  the already accumulated timeToPreviewShow.

[0] https://meta.wikimedia.org/wiki/Schema:Popups

Bug: T164256
Change-Id: I3fd253f1c2ff5fa8e24c9225060d728ffd8dfd27
2017-05-09 06:51:05 +01:00
Baha 6d95bbf630 Refactor and test renderer#createLayout
* Make function dependencies explicit;
* Only pass data that's needed, no more.

Bug: T133022
Change-Id: Ia973bd9636a477a76db907a37e2a0689daf4e3fa
2017-05-05 14:20:46 -04:00
Ed Sanders 81a6d49e99 build: Update eslint to 0.4.0 and make pass
Change-Id: I7d301049f0c91f79e82f996b2dce770840c27e72
2017-05-05 05:43:16 +01:00
Baha 0b169321d8 QA: Test renderer#bindBehavior
Bug: T133022
Change-Id: I3b0b199010f0460d83f80c57e4bab8e3606a4b4f
2017-05-04 15:48:10 -04:00
Sam Smith 742f341e5c Hygiene: Tidy up QUnit references
Since Ieea378c9 all QUnit tests are run in Node.js and not in the
browser. Tidy up references to QUnit inside of the codebase and tooling.

Changes:
* Don't exclude src/processLinks.js in Istanbul code coverage reports.
* Don't test for window.QUnit in createSchema. We no longer run the risk
  of sending beacons while running the QUnit test suite as it's no
  longer run in the browser.

Bug: T160406
Change-Id: Ifb6adb84b8019dd69231b50af00bf978b708fc60
2017-05-04 15:53:44 +01:00
Baha d95badc614 QA: Test renderer#hide
Bug: T133022
Change-Id: I752c4266b6be1909a3265d8292e7c5229e5724fb
2017-05-03 13:26:19 -04:00
Baha eb9ceb6238 QA: Test renderer#createPreview
Bug: T133022
Change-Id: Ied8e798851d5aebf2910aba5866f8a801c20923b
2017-05-03 13:23:43 -04:00
Baha a46a5fc02f QA: Test renderer#createEmptyPreview
Bug: T133022
Change-Id: If93d2c679bd7edcf98946e1cb688bc5c3744f69b
2017-05-03 13:21:24 -04:00
Baha ccca8b7a90 QA: Test renderer#createThumbnailElement
Bug: T133022
Change-Id: I7909c208e3d69b2ee510dcd8bfbcafc0118397ca
2017-05-03 16:25:28 +00:00
Baha 7f89341cc6 QA: Test renderer#getClasses
Bug: T133022
Change-Id: If3b8888a948f6e14d8847fdc1d4ea16e42329bb8
2017-05-03 16:24:18 +00:00
Baha 872691e293 QA: Bring back renderer#renderExtract tests
The tests were removed in Iae0a78d0b8a13353de70794b67387f2c3bab44c6.
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.

Some of the tests have been removed as the functionality covered by
those were moved to model.js in I20f29657fcf94101a71ed13c0920508db71292ce.

Bug: T133022
Change-Id: I7b20324dd5fe8a428cdd96959b65bc82d44fb515
2017-05-02 12:09:11 -04:00
Baha 234282d1a2 QA: Test renderer#createPokeyMasks
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.

Bug: T133022
Change-Id: I3ecbfb9bb3ac9c63fdd40df502796748c62949fe
2017-05-01 11:00:42 -04:00
Baha 2030cb6b06 QA: Bring back renderer#getClosestYPosition tests
The tests were removed in Iae0a78d0b8a13353de70794b67387f2c3bab44c6.
The ultimate goal is to refactor the renderer code and make it
testable, but before doing so we need to add tests to cover the existing
code. This will give us confidence that we won’t accidentally break
anything when we refactor.

Bug: T133022
Change-Id: I897276a1a953f6be62e4c2d4a24e0f22fc6ef141
2017-05-01 10:56:41 -04:00
joakin 1da916ee73 Tests: Refactor processlinks test
With the final goal to remove the real mw stubs, move the processLinks
tests away from test cases, and split getTitle to be tested standalone.

Supporting changes:
* Move getTitle out to it's own module
  * Will be tested separately in a followup commit
* Remove global declaration of mw.popups.processLinks (not needed any more)

Bug: T160406
Change-Id: Ieebd1257a2476081c67a318d3f05dffa1d3b9bdd
2017-04-28 22:39:49 +01:00
jenkins-bot 1adcc83af8 Merge "Track and discard duplicate enqueued events" 2017-04-28 20:06:13 +00:00
jenkins-bot 5e10affd8f Merge "Remove isLoggingEnabled with Null Object pattern" 2017-04-28 19:46:16 +00:00
Sam Smith 79f3b318d0 Track and discard duplicate enqueued events
The eventLogging change listener is responsible for ensuring that the
internal state of Page Previews matches its external state (that
perceived by the user and UA). It does this by logging events with
ext.eventLog.Schema#log. This makes it the perfect place to track and
discard duplicate events enqueued by the Page Previews codebase observed
in T161769.

Make the change listener track events that it's logged by storing hashes
of the dynamic parts of them in memory. If the eventLogging change
listener sees the same event more than once, then it discards it and
increments a counter in StatsD.

This behaviour should be enabled for a matter of days as we should see
whether the duplicate events are being enqueued by the Page Previews
codebase immediately.

Bug: T163198
Change-Id: I6a38a2619d777a76dd45eb7300079e1f07b07b12
2017-04-28 15:12:40 +01:00
Sam Smith 3d0899c0a0 Remove isLoggingEnabled with Null Object pattern
The statsv change listener depended on both the analytics tracking
function and whether it should log metrics to StatsD. We can simplify
the behaviour of the change listener by passing in a function which
doesn't log metrics to StatsD if such logging is disabled.

The change listener is now more isolated from other components.
Moreover, sharing the analytics tracking function with other components
is simpler as there's no repeated code.

Bug: T163198
Change-Id: Ibf4785fa4c27c1ad4739f02410f57412f56ff481
2017-04-27 14:29:04 +01:00
Sam Smith 939ebe386d Optimise SVGs with svgo
... by running the following:

    svgo --disable removeXMLProcInst --pretty --folder images/
    svgo --disable removeXMLProcInst --pretty --folder resources/ext.popups.images/

Also, add an XML declaration to resources/ext.popups.images/close.svg so
that it's recognised as an SVG by libmagic. This isn't strictly an issue
because of the way the SVG is served by the ResourceLoaderImageModule RL
module but it's consistent with the other SVGs.

Change-Id: I10b2286d6577701ba3b9a8651d5165fa81b8d293
2017-04-26 17:15:45 +01:00
jenkins-bot 1a8b147f97 Merge "Don't show preview if user has abandoned link" 2017-04-24 21:33:12 +00:00
jenkins-bot 1e4a95cb73 Merge "Improve illustration on disable popup" 2017-04-24 18:45:37 +00:00
Baha 764fb36017 Improve illustration on disable popup
The updated illustration makes it easier to find the footer link
to enable Previews.

Bug: T154330
Change-Id: I8b6cbe9e2f002eaff06a293643dc0f4aa1d5a641
2017-04-24 19:26:26 +01:00
Sam Smith 7f91068ecf Don't show preview if user has abandoned link
If the user dwells on a link for long enough, then the gateway makes a
request, which is allowed to complete regardless of whether or not the
response is required.

If the user abandons the link but the request completes before the
abandon completes - currently, ABANDON_END_DELAY is 300 ms - then the
preview will be rendered temporarily.

Fix this by not rendering the preview if the user has started to abandon
the link.

Bug: T163350
Change-Id: I154dde4e3ccaed3d11cb023c85c44451fc0ad957
2017-04-24 15:35:07 +01:00
jdlrobson b0e059b9de Generate cog icon via ResourceLoaderImage module
* Merge mwe-popups-icon with mwe-popups-settings-icon
* Remove PNG - now generated by the ResourceLoader module
* Adjust popup footer paddings/widths and store them in variables
* RTL and LTR compatible

Bug: T133956
Change-Id: I14ccd7b6731e9ec49f9959411fd17f7c9fdf43be
2017-04-21 16:23:21 -07:00
Sam Smith 5c5872d31f Don't occlude link when preview is above mouse
When the user dwells on a link and there's enough room to display a
preview above it, then the preview should rest atop the link rather than
above the mouse.

Bug: T161366
Change-Id: Ia7266f6e5c272817581bdbcb3710429b266556e4
2017-04-20 16:41:19 +01:00
Sam Smith 8c611d069f actions: Conditionally dispatch ABANDON_*
If the user CmdOrCtrl+Clicks on a link, then the link will remain
focussed. If a preview is shown, clicks another element on the page, and
then there'll be no token to include in ABANDON_* with.

Bug: T162924
Change-Id: Ie2237aa55ea9a11070498b66c73b8bf1898d8d44
2017-04-18 20:03:58 +01:00
Sam Smith 2c171d7f25 reducers: Don't destroy interaction on LINK_CLICK
I09d8776 introduced a bug in the eventLogging reducer where reducing
LINK_CLICK would remove any accumulated interaction state but not close
the interaction.

Update the associated tests so that they correctly test the reduction of
this action.

Bug: T162924
Change-Id: Ia03e719c228ee96f279c1fa89252dc6b6371a8e9
2017-04-18 19:43:31 +01:00
Sam Smith bb43c9c2e6 reducers: Update eventLogging documentation
Bug: T159490
Change-Id: Ica8363ffb166cf4e1583b481a7055f6d1678ccf4
2017-04-17 15:06:54 -07:00
Sam Smith 56aeeccb0d reducers: Make LINK_CLICK finalize but not close
... the interaction.

Following on from Iccba3c4c, LINK_CLICK shouldn't be considered the end
of an interaction because the link or preview can still be abandoned by
the user. Unlike ABANDON_END and LINK_DWELL, therefore, LINK_CLICK
musn't destroy the interaction but indicate that no more events should
be enqueued for the interaction.

More concretely, if the user has clicked the link or the preview, then
a "dwelledButAbandoned" or "dismissed" event shouldn't be logged.

Changes:
* Distinguish between "finalizing" and "closing" an interaction, where
  the latter is the current behavior of ABANDON_END, LINK_DWELL, and
  LINK_CLICK, in the eventLogging reducer and associated tests.
* If the interaction is finalized, then either the "dwelledButAbandoned"
  or "dismissed" events shouldn't be logged.

Bug: T162924
Change-Id: I09d8776da992053f89a77508e29a7cde3cfeeac6
2017-04-17 15:06:00 -07:00
Sam Smith d55e8b9a17 Don't load entire codebase in QUnit tests
As noted in T160406, the only QUnit tests that requires a running
MediaWiki instance is the test for mw.popups.processLinks. The function
itself is isolated from the rest of the codebase.

Now, as noted in T162876#3182198, during boot the
ext.eventLogging.Schema module is loaded asynchronously with
mw.loader.using. Since boot is unconditional and happens ASAP this
happens when the tests are loaded and run.

In the short term this can be avoided by not making the tests depend on
the entire codebase. The long term solution is laid out in T160406.

Supporting changes:
* Bundle assets with webpack@2.4.1.

Bug: T162876
Change-Id: If1ee1853ba7a9b2a66b24bb93b4e6062b92b0dba
2017-04-14 17:59:32 +01:00
Sam Smith 83e32c255d reducers: Make LINK_CLICK finalize interaction
... in the eventLogging reducer.

Like ABANDON_END, the LINK_CLICK should be considered the end of the
interaction in the context of the EventLogging instrumentation, i.e. no
events should be logged after the user clicks the preview or the link.

See T159490#3172692 for additional context.

Bug: T159490
Change-Id: Iccba3c4c2b6121016ff7923c11b1622bc046ad6b
2017-04-12 11:45:04 +01:00
Sam Smith 76b8c18ca5 reducers: Make PREVIEW_SHOW require a token
Like the FETCH_COMPLETE and ABANDON_END actions, the PREVIEW_SHOW action
was delayed but not conditionally reduced. As ABANDON_END is delayed,
there's a potential for a race and if ABANDON_END is reduced before
PREVIEW_SHOW, then there's no interaction to reduce the action into,
which causes an error, e.g. T159490#3165276 and T162373. Making
PREVIEW_SHOW require a token stops the error occurring in this scenario.

An alternative would be to clear the timeout created in
ext.popups.Preview#show in #hide. However, this would be inconsistent
with actions#fetch and actions#abandon.

Bug: T159490
Change-Id: Ibd2c0c6f45e4392582cc6ed08517f6ca1146d57a
2017-04-11 14:06:04 +01:00
Sam Smith b3b746226a Hygiene: DRY up eventLogging reducer
Extract the repeated token testing for the FETCH_COMPLETE and
ABANDON_END actions.

Bug: T159490
Change-Id: Iebb167c4039f1685bb98ded7cbe5bb3ef3275dd7
2017-04-11 14:05:50 +01:00
Sam Smith 77943704f8 actions: Add token to PREVIEW_SHOW
Mirroring all other actions that are dispatched after some delay, add
the token to the PREVIEW_SHOW action.

Supporting changes:
* Pass the token to ext.popups.Preview#show so that it can be passed to
  actions#previewShow.

Bug: T159490
Change-Id: I128fd56e770ed09d5d0dc55db73d11b013049c79
2017-04-11 09:26:22 +01:00
Sam Smith 87be4be855 reducers: Reduce FETCH_COMPLETE if token matches
... instead of using the active element.

In the case of the eventLogging reducer, this fixes scenario 4.1 from
T159490#3150331, which was caused by late FETCH_COMPLETE actions being
reduced regardless of whether interaction had been finalised or a new
interaction had started.

Bug: T159490
Change-Id: If9d718625b0302ea2f75a778005643b4eef62bde
2017-04-07 14:06:14 +01:00
Sam Smith d3d9100637 actions: Add token to FETCH_COMPLETE
Bug: T159490
Change-Id: Ic71dd3ce5e7933273f84a9a64d41e7f3a4cb03f4
2017-04-07 14:06:11 +01:00
Sam Smith 52d128863e actions: Correctly delay FETCH_COMPLETE
I496fe317 caused a regression where the FETCH_COMPLETE was delayed for a
total of 650 ms rather than 500 ms. This is evidenced by a 150 ms step
in the median Time To Preview immediately after today's (Thursday, 6th
April) MediaWiki train [0].

[0] https://grafana.wikimedia.org/dashboard/db/reading-web-page-previews?refresh=1m&orgId=1&from=1491505806387&to=1491507027263

Change-Id: Ic31656208671766f2c08cfaf55babba64455a614
2017-04-06 21:17:44 +01:00
Baha 5b613b1698 Disable Previews when Navigation Popups Gadget is enabled
Only showing a preview is disabled, EL and other stuff should work.

Bug: T160081
Change-Id: Ia837816081781dcaea9a8b4c722f8df5b3e3ca03
2017-04-06 13:11:17 -04:00
Baha e3fde6e360 Handle RESTBase 404
Treat these responses not as an API failure. Show a generic
preview whenever the server responds with a 404.

Bug: T160744
Change-Id: Id6169d9d4c7493f5b6511cc78fe65d448cdadc03
2017-04-06 18:03:11 +02:00
jdlrobson f80acb978b renderer: Pass event to behavior for processing
I6d9ff52b introduced a regression where if a logged out user clicks the
settings cog then an error is thrown.

For now, passing the event to the behavior for further processing is
required. However, it's clear that this makes the
ext.Popups.PreviewBehavior abstraction leaky.

Bug: T162324
Change-Id: I9dea04eb7435f9349e60d477f5701ec5dd655ebd
2017-04-06 10:44:37 +01:00
Bartosz Dziewoński 7967ab77ee Properly create the <svg> and <image> elements for thumbnails
We have to be careful about the namespaces here, and then we don't
need the awful `.html( .html() )` hack. (I honestly have no idea
why that even worked for some browsers, it really shouldn't have.
The comment next to it is wrong.)

* Construct the 'svg:svg' element with the right namespace
* Set 'xlink:href' attribute on 'svg:image' element with the right namespace

Doing this correctly makes the thumbnails work in Opera 12, and it also
works as before in (at least) Chromium 57, Firefox 53, IE 11 and Edge.

I can't find out what version of Safari the other hack here was
supposed to apply to, but the code was wrong in both cases, and the
hack was mistakenly also applied to modern Chromium.

Useful resources for dealing with SVG embedded in HTML while scripting:

* http://stackoverflow.com/questions/6701705/programmatically-creating-an-svg-image-element-with-javascript
  * http://jsfiddle.net/UVFBj/8/
* https://www.w3.org/Graphics/SVG/WG/wiki/Href#xlink:href

Bug: T161799
Change-Id: I30b2a1291811296424018e013bd07055ae7551d7
2017-03-31 22:39:28 +02:00
Sam Smith 1e199b67f0 actions: Simplify delaying FETCH_COMPLETE
Require that two promises are resolved (or one is rejected) before the
FETCH_COMPLETE action is dispatched. The first promise represents the
gateway request and the second represents an arbitrarily long delay. If
the first resolves before the second, then there'll be a delay until the
second resolves; whereas if the first rejects, then there's no delay.

Change-Id: I496fe317337745c593594efff26688c46d661bf3
2017-03-30 17:48:58 -07:00
Sam Smith 6042000eb1 actions: Don't mix delay into FETCH_COMPLETE
Mixing in the delay was introduced in If3f1a06f so that the total RTT
for an API request could be calculated. Now that the FETCH_END action is
dispatched when the gateway request ends and not when the preview model
is resolved, this additional information (state) is redundant.

Change-Id: I7e6ffe0945ffedd9425525fa7da855e729d50b77
2017-03-30 17:48:05 -07:00
Sam Smith 3646b04876 actions: Dispatch FETCH_END
... when the gateway request ends, per I62c9cb04.

Change-Id: Ifbed65d6b97877e859e81f256fa44344a64fc64f
2017-03-30 17:47:39 -07:00
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
jenkins-bot cb4cdc868a Merge "build: Make webpack config compatible with v2.3.0" 2017-03-23 15:13:17 +00:00
Sam Smith 29df4b11be build: Make webpack config compatible with v2.3.0
webpack@2.3.0 expects the output.filename config variable to be a
relative path.

Change-Id: If82a0d6d69e0400c1d5b5722ea7dd6bd55160204
2017-03-22 11:32:35 +00:00
Ed Sanders c46b1c8db3 build: Update stylelint to 0.4.1
Change-Id: I143662a36bf5d79cfc5b35ec62c3473f91afd14c
2017-03-20 14:50:42 +00:00
Piotr Miazga e40e7a666b Fix horizontal gradient for RTL page previews
Changes:
 - added @noflip to every .mw-popups-extract:after declaration

Bug: T160614
Change-Id: I45ccb0c64e110ab0fc24e0e4eca31393cbef36d2
2017-03-17 22:56:54 +01:00
jenkins-bot 7c97e92ddf Merge "renderer: Really ensure images don't overflow" 2017-03-17 15:37:52 +00:00
Sam Smith f6db973fb9 renderer: Really ensure images don't overflow
... container.

I19e67ae4 hide the overflowing parts of the SVG image element in IE9-11
for a number of cases but not all of them, e.g. see T139297#3089714.
Moving the overflow: hidden property to the SVG element fixes the above
case and is clearer.

Tested in IE9-11 on Windows 7, and Chrome (56.0.2924.87) on macOS Sierra
(10.12.3).

Bug: T139297
Change-Id: I9c397d7333766b40abbf14b6ade96788f5023dfa
2017-03-17 10:58:27 +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
jenkins-bot 36fd908c2d Merge "renderer: Ensure settings cog visible in IE9-11" 2017-03-08 10:22:29 +00:00
jenkins-bot f6e1f74226 Merge "renderer: Ensure images don't overflow container" 2017-03-08 10:21:48 +00:00
Sam Smith 0cf00b04fa renderer: Ensure settings cog visible in IE9-11
Following on from I19e67ae4, IE9-11's treatment of SVG elements that
overflow their containers means that the truncating pseudo-element and
the settings cog is occluded in a portrait-mode preview.

"Pad" the extract horizontally using a margin to force the SVG element
into the correct position.

Bug: T156800
Bug: T139297
Change-Id: I0da6af6d4cbcc69c6465b37714856e59199ae6e4
2017-03-08 09:40:37 +00:00
Sam Smith 57f3d4865e renderer: Ensure images don't overflow container
Unlike modern browsers, IE9-11 (at least) don't hide the overflowing
parts of an SVG image element. Make this explicit by adding the
overflow: hidden property to the preview's container element.

Bug: T156800
Bug: T139297
Change-Id: I19e67ae4584d90c02dc5a2dd1c8bdb5773cd2283
2017-03-08 09:40:37 +00: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
jenkins-bot 7a54c13ec4 Merge "truncation: Fix fade direction for RTL previews" 2017-02-27 17:12:14 +00: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
Sam Smith 84a60c4db7 truncation: Fix fade direction for RTL previews
CSSJanus v1.1.3 doesn't appear to flip the "to right" argument for the
linear-gradient CSS function. As with the position of the truncating
element in I0d50a8b5, the direction of the gradient doesn't vary with
the text direction of the current page but that of the target page.

Bug: T158858
Change-Id: I4b6fcf68bdf57722348513f12c7b19f80b2545c4
2017-02-27 11:51:40 +00:00
Ebrahim Byagowi b64260b1cc i18n: Fix popups fade on RTL
Bug: T158858
Change-Id: I0d50a8b55ebdfb8167e36ba520fd6e7cfd4be717
2017-02-27 08:59:53 +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
jenkins-bot ade818e89e Merge "Resize thumbnails images returned by REST endpoint" 2017-02-17 00:30:28 +00: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
Derk-Jan Hartman b839b29008 Position image correctly inside the card
inline-blocks need to be forced to be aligned correctly on the baseline

Bug: T153840
Change-Id: Iec45caa4483b36b01261309e4895c71e3fc2e675
2017-02-16 23:35:15 +00:00
joakin fa2b10a2e7 Hygiene: Move build/ext.popups/ to src/
This way, src contains sources, and dist contains distribution files.

Also, add some documentation about the folders in the README and an adr.

Change-Id: Ie0b9f6475b8423b90e927633d883bde3cd5d5e4d
2017-02-14 09:59:59 -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
joakin 908d4eed7e Hygiene: Include sourcemaps in the repo in the compiled assets folder
So that production code can be debugged properly. Independent source
maps files aren't loaded until the developer tools are open.

Change-Id: Ic8c3c10315d3a3be0f42505834005a3cab77d130
2017-02-13 17:59:03 -08:00
jdlrobson f0f462ee0d Redux and redux-thunk now packaged inside ext.popups
Until this appears in core it makes sense to package it up as
part of ext.popups resource loader module.

Version numbers from npm are pinned to exact versions to control the
upgrade path of the libraries instead of leaving it to semver on
developers machines.

Bug: T156333
Change-Id: I33368ecc3c8e911d96f846669bcd831c182749f2
2017-02-13 14:15:30 +01:00
jdlrobson 20e7564bed Webpack: Complete transition to webpack for ext.popups module
boot.js is renamed to index.js and popups is now bundled
up inside popups.js

To support qunit tests, the library is still exposed as
mw.popups with a FIXME to remove later when it is no longer
necessary

Bug: T156333
Change-Id: Ieb6b4b0344af2701f99ef0fcc786d2378fd2fceb
2017-02-13 13:48:52 +01:00
jdlrobson 93b2428895 webpack: Build gateway via webpack
Bug: T156333
Change-Id: Ifbb696c5e615679c2c61bde7df7224bd5e55fcdb
2017-02-13 13:42:22 +01:00
jdlrobson c6ca8f173f Webpack: Build previews with webpack
Change-Id: Ieb9e3bf1bee550e4234a5796c0b51405ba458b1f
2017-02-13 13:42:22 +01:00
jdlrobson 915bf23627 Hygiene: Reducers packaged via webpack
Bug: T156333
Change-Id: I14b0a9faf063ac7e89b7a012fb47d27ef4ed9d82
2017-02-13 13:42:22 +01:00
jdlrobson 49df4b9572 Tooling: Begin to use webpack for JS code generation
Generate changeListeners via webpack
We now use a build folder to build the JavaScript for
our ResourceLoader modules. This is the first change
in a line of changes.
A source map is provided for debug support.

Bug: T156333
Change-Id: I771843d1ddb4b50adedc3fa53b30c2f1d8a76acb
2017-02-13 13:42:22 +01:00
jenkins-bot fa0426e008 Merge "Hygiene: Rename isEnabledByUser to shouldSendModuleToUser" into mpga 2017-02-10 18:57:27 +00:00
jdlrobson 64e7bfd620 Hygiene: Rename isEnabledByUser to shouldSendModuleToUser
Sometimes we make choices on the users behalf if we don't have
sufficient information, so this name is more applicable.

If beta features is disabled and the user is anon they have not
explicitly opted in.

Change-Id: I5d816f569fc54f8bf74d6e5a06246b7fa7036e06
2017-02-10 10:07:28 -08:00
jenkins-bot 398ffb0e43 Merge "Hygiene: Add to mw.popups in only one place" into mpga 2017-02-09 10:53:41 +00:00
jenkins-bot cd9451e538 Merge "Hygiene: Split and organize the gateways" into mpga 2017-02-08 21:20:18 +00:00
jenkins-bot d7b5f56665 Merge "Hygiene: Use factory functions instead of classes" into mpga 2017-02-08 19:38:53 +00:00
joakin 955189230a Hygiene: Split and organize the gateways
* Create new namespace mw.popups.gateway to contain them
* Create folder resources/ext.popups/gateway/
* Split gateway{,.test}.js into gateway/{mediawiki,rest}{.test,}.js
* Extract stateless functions out of factory scope now that there are no
  name collisions between the factories.

Bug: T123445
Change-Id: Ib256871c3e4cfe3f13361cb66d4e9a67e9823c7b
2017-02-08 18:58:46 +01:00
joakin 557fd113c8 Hygiene: Use factory functions instead of classes
Instead of using constructor functions, use factory functions to
generate the gateway objects, because of:

* Consistency on the approach in the repository, no constructor
  functions are used, the factory function pattern prevales.
* Real private data with closures
* No use of `this`, which is dynamic and unbound in JS and a source of
  errors. In contrast, by using a factory function, the
  function/methods-of-the-object are tightly bound to the internal data.

Additional changes:
* Specify more the type of createRESTBaseGateway's parameter to improve
clarity on the intent of the parameter
  * From: @param {jQuery} api
  * To:   @param {Function} ajax function from jQuery for example

Supporting documentation:
* https://medium.com/javascript-scene/javascript-factory-functions-vs-constructor-functions-vs-classes-2f22ceddf33e

Change-Id: Iacbb098b646843a01f459b15343057e2e4851d35
2017-02-08 18:28:21 +01:00
jdlrobson 6cc50d7a78 Hygiene: Add to mw.popups in only one place
Prepare to shift to a JS bundler by placing definitions at bottom
of page (these are currently assignments to mw.popups). In future
these are easily switched to module.exports line

Change-Id: Iaa5d78fee1cdb6fc572d2f1781f1d4fb59475e84
2017-02-07 14:55:13 -08:00
jenkins-bot bc7ae795c3 Merge "Add support for RESTBase endpoint consumption" into mpga 2017-02-07 18:48:18 +00:00
Baha 9764f7fe64 Add support for RESTBase endpoint consumption
* Introduce the new config variable `PopupsAPIUseRESTBase`.
* Create a gateway interface that exposes the getPageSummary
  method. Both MediaWiki API gateway and RESTBase gateway
  implement this interface.

Bug: T123445
Change-Id: I71a8a848f3143fa4a0dfd4ca182ee086903110bc
2017-02-07 12:36:11 -05:00
Sam Smith 75aaf7a23e Don't log editCountBucket when user is logged out
Bug: T156893
Change-Id: I3cdfbe0c9872407b900f0676396b0bc3102a43fd
2017-02-06 11:58:18 +00:00
Sam Smith 83821e105c Hygiene: Extract helper from eventLogging reducer
Extract the getBaseData function from the eventLogging reducer so that
changes in the shape of the base data will be isolated to that function.

Additional changes:
* Clarify that the eventLogging reducer is specific to the Popups
  schema. Add a TODO for revisiting the name, at least.

Bug: T156893
Change-Id: I99c102d3a1dbf77d526e5ab33e99929f351f5768
2017-02-06 11:58:11 +00:00
jdlrobson eea509637a Remove unused code in repo
The existence of these files was causing me much confusion as I thought
they were still being used in the mpga branch.

If this is note the case they should be removed to make the repo easier
to understand.

Changes:
* Move the ext.popups.animation.less into ext.popups since that's where
its shipped.
* Remove files which are not being used.
* Since csslint now runs on ext.popups.animation.less fix the linting
issues (previously linting was not occuring)

Change-Id: Iae0a78d0b8a13353de70794b67387f2c3bab44c6
2017-02-02 16:18:41 -08:00
Sam Smith ab75283392 model: extract -> page
In I6bbff6a5 a preview was assigned the "extract" type if it had an
extract whereas it should've been assigned the "page" type.

Bug: T152004
Change-Id: I5e0cdc8f528647f1e20873753be29c6b85b3a384
2017-02-02 17:06:46 +00:00
jenkins-bot 8c96564915 Merge "Add close button to settings dialog" into mpga 2017-02-02 14:21:54 +00:00
jdlrobson 09ccfa9a13 Remove last modified line
Note this is a breaking change in the sense that it breaks the function
signature for createModel which no longer needs to be passed the last
modified time. Given this change is in the mpga branch at current time
and hasnt been exposed for public use I have deemed this okay.

The change removes tests relating to the last modified bar, removes
lines in the templates and updates existing tests and code to reflect
the new function signature for create model.

Settings icon is floated to right which will be flipped for RTL
users.

Bug: T137775
Change-Id: I7737e37d956c62f1f1c0694d7a25a58d91651f4d
2017-02-02 11:28:11 +00:00
jdlrobson 6fab3c9ca4 Add close button to settings dialog
* Load the close icon (previously was not loaded)
* Take opportunity to simplify selectors in setting panel and remove
unnecessary id selectors
* Correct size of header to be consistent with mediawiki ui icon

Note: there's a slight movement in the heading text's position
presumably due to inconsistiencies between the text
inside the Done/Save buttons

Bug: T154645
Change-Id: Icd978b45051c43f24ee1c9a8574a961b942082f1
2017-02-01 12:06:38 -08:00
jhobs cfa0a4dd55 reducers: Add preview type to dismissed event
Bug: T152004
Change-Id: I941ca556b914d0c009cc8e395660e4d299d08020
2017-01-30 10:07:47 +00:00
Sam Smith 2d42471ccd reducers: Add preview type to interaction state
The preview model is included in the FETCH_END action so, when reducing
that action, pluck the preview type from the model and add it to the
interaction state.

Change-Id: I0e51a85b4dada8f9a17ec4882e0c13b2c4382d24
2017-01-30 10:05:17 +00:00
Sam Smith 44c6d3d9b9 Add type to preview model
Per T152004, if the preview has an empty extract, then it's a "generic"
preview; it's an "extract" preview otherwise.

Bug: T152004
Change-Id: I6bbff6a507fe3e4f03b3a44f64d875e7dbb512df
2017-01-30 08:29:29 +00:00
Sam Smith 22654c3a87 Hygiene: mw.popups.Preview -> ext.Popups.Preview
The rest of the codebase creates and refers to types in the "ext"
namespace.

Change-Id: I83b537ce7a89f4a800a99c16e3f404570f21f9f8
2017-01-30 08:29:28 +00:00
Sam Smith 7ce7423639 Make gateway use preview model
Following on from I20f29657, simplify the gateway by making it return
instances ofext.popups.PreviewModel.

Supporting changes:
* data -> model in mw.popups.renderer#render and the
  create{,Empty}Preview helper functions.

Bug: T152004
Change-Id: I531f609056ae813ae5e6b1d53010d77be0799ec1
2017-01-30 08:29:24 +00:00
Sam Smith 622f2af318 gateway: Don't create new object for thumbnail
As noted in the review of I71a8a84, creating a new object to represent
the thumbnail is confusing. It's also somewhat wasteful.

Since both the MediaWiki API and RESTBase both agree on using "source"
as the key for the thumbnail's URL, it's reasonable to use it in the
gateway and renderer.

Bug: T152004
Change-Id: Ie1346d13b6e88004b817f5968448188ed83cfda4
2017-01-27 14:22:54 +00:00
Sam Smith e3ae38f966 Introduce preview model
After the gateway has made a request, it processes the page. This
processing both mangles and processes the MediaWiki API response. The
latter step contains two pieces of related business logic:

1. Deciding if a page has been modified recently.
2. Processing the extract/deciding if it's viable (see processExtract).

In order to support switching between the MediaWiki API and RESTBase and
to simplify the event logging reducer when it comes to instrumenting the
"type" of preview, extract this business logic into a model.

Bug: T152004
Change-Id: I20f29657fcf94101a71ed13c0920508db71292ce
2017-01-27 14:15:37 +00:00
jenkins-bot fe32ef8fd7 Merge "Add hovercardsSuppressedByGadget to logging" into mpga 2017-01-26 01:19:00 +00:00
Sam Smith 6cc6252704 Add hovercardsSuppressedByGadget to logging
I4c15296e introduced the wgPopupsConflictsWithNavPopupGadget client-side
config variable, an always-available flag that represents whether the
user has enabled the NavPopups gadget. This property has been missing
from the Popups instrumentation since I8a3f5835.

Changes:
* Add the value of wgPopupsConflictsWithNavPopupGadget to BOOT action
  and reduce it into the base instrumentation data.

Bug: T151058
Change-Id: I09924d0cb10e96b0a846940506ebfeaa7086cd17
2017-01-25 14:21:34 +00:00
Sam Smith ad5cb599f9 Fix page content language direction handling
Ieb00709e changed the names of the page content language related
variables returned by the API from langdir and langcode to
languageDirection and languageCode respectively without updating the
renderer.

Moreover, the extract truncation mechanism introduced in I2ec0c04
couldn't handle RTL languages.

Both of these issues were found while writing a test plan for Page
Previews.

Change-Id: Ied3dbd7cf82749198e792a2f9d2241582aeff25c
2017-01-25 11:19:09 +00:00
Sam Smith 51c6f2c8e5 Hygiene: Make settings reducer control footer link
Since the footer link part of the settings system - clicking the link
opens the settings modal - it seems appropriate that it should be
controlled by state managed by the settings reducer.

Bug: T146889
Change-Id: I3a8549dbf1952cd0556f663496c55de91acaf2c0
2017-01-17 06:40:36 -08:00
Sam Smith c51b33c8b7 Send blob to everyone
As before, we need to send the Page Previews blob that logged out users
can enable/disable previews.

Changes:
* Unconditionally add the ext.popups RL module to the output.
* Send the value of Popups\PopupsContext#isEnabledByUser to the client
  as $wgPopupsIsEnabledByUser.
* Make mw.popups#isEnabled return $wgPopupsIsEnabledByUser if the user
  is logged in.

Bug: T146889
Change-Id: Id2ccfaa81a327e222fff400f4a5f22f96c99ea31
2017-01-17 06:40:24 -08:00
Sam Smith c6a502954d Vary cog click behavior
The behavior of the cog varies when:

* The user is logged in/out.
* Page Previews is enabled as a beta feature.

Since the behavior of the cog doesn't vary per-preview, it can be
determined at boot time and passed to the renderer. However, in order to
keep the renderer stateless, we pass the behavior to it when a preview
is added to the DOM.

Changes:
* Add the mw.popups.createPreviewBehavior factory function, which
  returns an object that encapsulates how a preview responds to the user
  dwelling on it, abandoning it, and clicking on the cog.
* Invoke mw.popups.createPreviewBehavior at boot time and pass it to the
  mw.popups.changeListeners.render change listener, which then passes it
  to mw.popups.Preview#show.
* Make mw.popups.Preview#show responsible for binding event handlers
  and configuring the cog based on the behavior.

Bug: T146889
Change-Id: I39d7d0afd7b1fe896019a1b3a82ee907bfb20edd
2017-01-15 16:20:22 -08:00
jenkins-bot 1b7283a301 Merge "Hygiene: Rename SchemaPopupsSamplingRate" into mpga 2017-01-15 23:38:53 +00:00
Sam Smith 98b2dcf631 Hygiene: Rename SchemaPopupsSamplingRate
The SchemaPopupsSamplingRate config variable is inconsistently named and
even forwarded to the client as wgPopupsSchemaPopupsSamplingRate.

Changes:
* SchemaPopupsSamplingRate -> PopupsSchemaSamplingRate
* Forward PopupsSchemaSamplingRate to the client as
  $wgPopupsSchemaSamplingRate.

Bug: T146889
Bug: T146434
Change-Id: I80d6a0abccf462e2eb0fd96af6849b5e82b49c26
2017-01-15 11:01:00 -08:00
Sam Smith 787660f956 actions: Merge LINK_ABANDON_* + PREVIEW_ABANDON_*
Per Id1339dc2, the LINK_ABANDON_* and PREVIEW_ABANDON_* actions can (and
should) be merged, as they are always reduced in the same way.

Change-Id: I71b30d4d2774deb4efea9e565f2ccd7383bf08c1
2017-01-15 09:55:36 -08:00
Sam Smith 49ce7643f0 Hygiene: Remove link element from LINK_ABANDON_*
Initially, the link element was added to LINK_ABANDON_* to determine
when an interaction should be reset. However, this wasn't enough to
protect against timing-related bugs, e.g. T154923. Now that tokens are
used to determine when interactions should be reset, the link element
can safely be removed from the LINK_ABANON_* actions.

Supporting changes:
* Update the return type of mw.popups.actions.linkAbandon in its
  DocBlock.

Change-Id: Iaaed7a4846af75f9c4051d7bc761022ea5b3f6cf
2017-01-11 14:33:46 -08:00
Sam Smith 1e2fd94734 Hygiene: Remove unused action types
PREVIEW_ANIMATING and PREVIEW_INTERACTIVE were introduced in
Ib495755b but never used.

Change-Id: Ib849960874f938185414ae74bc67d906f26aae0c
2017-01-11 12:56:32 -08:00
jenkins-bot a57a0a321d Merge "Fix linkTitleChangeListener" into mpga 2017-01-11 04:27:30 +00:00
Sam Smith e17e209003 Fix linkTitleChangeListener
I2ecf575b introduced a regression where the linkTitleChangeListener
wouldn't destroy the active link's title if it had been dwelled on
within 300 ms of the previous active link being abandoned.

The LINK_ABANDON_END action is dispatched after 300 ms and ignored if
the active link has changed.

Change-Id: If0c16afd6ca1c44f0e7eed497f62f0190005a619
2017-01-10 18:03:47 -08:00
joakin 21230742c0 Change preview to check *_ABANDON_END with tokens
... instead of using the active elements, and check also the user
dwelling flag to only act if the user is not dwelling.

Preview and link abandon end and start share now test cases, meaning it
can be collapsed into one action creator in a followup commit.

Change-Id: Id1339dc23e8f1b1b7514c769cd09f8bdadc759b4
2017-01-10 17:52:53 -08:00
joakin e49103061e Don't reset interaction info when dwelling back to a link
When dwelling back to a link, previously a new LINK_DWELL action would
be sent and the reducer would reset the interaction as if it was a new
one.

This shouldn't happen, since dwelling back to the active link (from
the preview, for example) is not a new interaction, and should not
create a new interaction or hide and show the preview.

With this change, the preview reducer has the notion of new or repeat
interactions, and only resets state on new ones, and only sets
isUserDwelling when dwelling on a the link of the current interaction.

Also when the interaction is repeat, we guard on the action creator and
don't trigger the wait or the fetch request.

Change-Id: I71cde81cbfe50b6f955e562e7e5b57d0f920fdb9
2017-01-10 17:44:37 -08:00
joakin 36518b1215 Don't hide preview when user dwells on link
... after dwelling on preview.

Currently, the asynchronous PREVIEW_ABANDON_END action wouldn't be
ignored if the user were to have dwelled on the link that the preview is
about. The action is only ignored if the user is dwelling on the
preview, i.e. if preview.isUserDwelling is truthy.

Set preview.isUserDwelling to truthy if the user is dwelling either on
the link or on the preview.

Bug: T154923
Change-Id: I8e6989790da5884992ee9dda9e4895e2e58e6694
2017-01-10 17:43:55 -08:00
Sam Smith af05386ea4 actions: Add token to PREVIEW_ABANDON_**
Change-Id: Ic84aa0c3144761b93beff9b7b82932647e504a5b
2017-01-10 16:47:19 -08:00
jenkins-bot 064437d7f7 Merge "actions: Conditionally dispatch LINK_DWELL" into mpga 2017-01-10 19:52:42 +00:00
jenkins-bot f3618e78bd Merge "actions: Add token to LINK_ABANDON_*" into mpga 2017-01-10 19:38:11 +00:00
jenkins-bot 66d325cad7 Merge "reducers: Store interaction tokens" into mpga 2017-01-10 19:37:28 +00:00
jenkins-bot 5ad4edf53d Merge "Hygiene: interactionToken -> token" into mpga 2017-01-10 19:34:52 +00:00
Sam Smith df0f175d72 actions: Conditionally dispatch LINK_DWELL
Change-Id: I9046296f061677674f9ad991ec6dacd1dd9db65c
2017-01-10 10:12:03 -08:00
Sam Smith 4d736b9a78 actions: Add token to LINK_ABANDON_*
Change-Id: I7f6187f6c0249c112a08163a343d35ee75cf85e8
2017-01-10 09:50:58 -08:00
Sam Smith 46cd38849e reducers: Store interaction tokens
Consider the user repeatedly dwelling on and abandoning a link. Testing
whether or not the element has changed before modifying state is not
sufficient. Fortunately, we already generate a unique token and include
it in the LINK_DWELL action for use in the eventLogging reducer - the
so-called "interaction token".

Change-Id: Iae9544055f7eccb4f7f071797eb360bc3456861a
2017-01-10 09:50:58 -08:00
Sam Smith f26768afe8 Hygiene: interactionToken -> token
... for brevity.

Change-Id: I03626728fae36176cec0eb844efb92303377bfbf
2017-01-10 09:50:57 -08:00
jenkins-bot 4f6f74035c Merge "Hygiene: Remove hidden preview from DOM" into mpga 2017-01-10 17:47:22 +00:00
jenkins-bot b081b0aa0a Merge "Consistently delay showing previews" into mpga 2017-01-10 17:42:47 +00:00
joakin 84043e86f9 Fix checkin tests to use fake timers
* To speed the time the tests spend running
* To make them reliably execute and not depend on magic numbers

Additional changes:
* Use mw.now instead of Date on checkin.js
* Split big setVisibleTimeout test into several smaller ones
  * Extract helpers for the tests

Change-Id: I9d3233fccf6de0997f968d096e375df996e87786
2017-01-10 09:31:32 -08:00
Sam Smith 1632697c2a Hygiene: Remove hidden preview from DOM
After the preview fade out animation has completed, then the preview
element should be removed from the DOM as it'll never be used again.

Supporting changes:
* Add the preview element to the DOM in ext.popups.Preview#show so that
  it mirrors removing it from the DOM in #hide.

Change-Id: I6b3adc962aa13fbd46dce5f4c4f741299e43c6d9
2017-01-09 15:00:10 -08:00
Sam Smith cbdcf7d353 Consistently delay showing previews
While I1bf953b2 fixed the typo in FETCH_START_DELAY, it didn't do
anything to address the inconsistency in the time it takes to render a
preview (preview delay).

If the gateway result resolves before the target preview delay (500 ms)
is met, then delay dispatching the FETCH_END action until it is. If,
however, the gateway result resolves after the target preview delay,
then dispatch the FETCH_END action immediately.

Supporting changes:
* Expose the fetch action to ease testing it and to help tidy up the
  mw.popups.actions.linkDwell tests.

Change-Id: Idf02343a0417cfbb33093a81d97ecebc5c1c7379
2017-01-09 15:00:02 -08:00
Baha 4afa1958a0 Add reading depth
Use schema revision 16163887.

Add the 'checkin' action, which is accompanied by the 'checkin'
property. The action is logged at the following seconds
(Fibonacci numbers) after the page loads: 1, 2, 3, 5, 8, 13, 21,
34, 55, 89, 144, 233, 377, 610, 987, 1597, 2584, 4181, 6765.
The `checkin` property contains the values listed above.

Bug: T147314
Change-Id: Ib9ec7bd0e60aa34a04e32222b025347f6ee31794
2016-12-21 15:08:20 -05:00
jenkins-bot 397d3e8bc2 Merge "Hygiene: Make mw.popups#isEnabled return boolean" into mpga 2016-12-16 19:48:10 +00:00
jenkins-bot 7e64a87d7f Merge "Hygiene: Remove experiment-related code" into mpga 2016-12-16 19:48:09 +00:00
jenkins-bot 4fe1cd9f69 Merge "Redefine when Page Previews are enabled" into mpga 2016-12-16 19:48:08 +00:00
jenkins-bot 2135c85106 Merge "Remove jshint/jscs, add eslint and stylelint" into mpga 2016-12-16 12:42:26 +00:00
joakin 4ca333e577 Remove jshint/jscs, add eslint and stylelint
...with the wikimedia presets.

For automatically fixing most of the JS lint problems run

      grunt eslint:fix

Some rules of stylelint were disabled given they cause problems with
existing popups code (like no id selectors for example).

Change-Id: I2153047c3ddbea50572dd329989088bb20787515
2016-12-16 13:35:34 +01:00
jenkins-bot fc75d9ec0d Merge "Make API requests after 50 ms" into mpga 2016-12-16 11:07:19 +00:00
Sam Smith b2bd1ebdf7 Hygiene: Make mw.popups#isEnabled return boolean
Action changes:
* Update the BOOT action to include isEnabled and update the associated
  tests.

Reducer changes:
* Make the preview and eventLogging reducers handle the BOOT action's
  new shape.

Bug: T152687
Change-Id: I3fa2098269a32912eda99ceb8f13887688a14c15
2016-12-16 10:59:47 +00:00
Sam Smith 6bd082920f Hygiene: Remove experiment-related code
Changes:
* Remove $wgPopupsExperiment and $wgPopupsExperimentConfig config
  variables.
* Remove $wgPopupsExperimentIsBetaFeatureEnabled client-side config
  variable.
* Remove MakeGlobalVariablesScript hook handler as it's redundant.

Bug: T152687
Change-Id: Id25cd7c16da14b7ce9bad502565de9ff6d29434e
2016-12-16 10:59:47 +00:00
Sam Smith a6423fbc75 Redefine when Page Previews are enabled
The Page Previews A/B test was disabled in T144490. After the changes
for that task had been deployed we discovered that the old definition
for when previews are enabled was invalid.

Previews are enabled:
* If the beta feature is enabled and the user has enabled the beta
  feature.
* The user hasn't disabled previews via the settings modal.
* And soon, if the beta feature is disabled and the user hasn't disabled
  previews via their user preferences (see T146889).

Changes:
* mw.popups#createExperiment -> mw.popups#isEnabled
* Make mw.popups#isEnabled act like a factory to maintain backwards
  compatibility.
* Update the associated tests.

Bug: T152687
Change-Id: I713a90dfb29866b27738e7d19e8a22518a12d417
2016-12-16 10:35:55 +00:00
joakin 212dcf1d55 Update the settings enabled radio with the enabled state
When opening the settings dialog, the form didn't represent the current
enabled status of the application. This change makes sure that every
time the form is shown, the form state represents the current
application state.

If the application is enabled, then the 'simple' (Enabled) option is
selected on the form. If it is disabled then if there's Navigation
popups, it selects 'advanced' (Advanced popups), if there are not, it
selects 'off' (Disabled).

Changes:
* Change listeners
  * Set the form state every time the form is going to be shown
    * Update tests
* Supporting changes
  * Add settingsDialog#setEnabled which updates the DOM form based on the
    enabled flag and the navpops state
  * Extract isNavPopupsEnabled function in settingsDialog.js to be used in the
    form creation and also when setting the form enabled state
  * Add test verifying changeListeners#settings shows and sets enabled state
    properly when showing the dialog more than one time

Change-Id: Ic660f48d9a78e47c09a192ab86681196d2e01d61
2016-12-16 10:29:34 +00:00
joakin 8631a183de Sync enabled preview state to storage on change
Changes:
* Generalized previewCount change listener to syncUserSettings
* Added state.preview.enabled to be saved on change

Change-Id: I403a490fee9c8e125175996ba30c63c232b5598b
2016-12-16 10:27:24 +00:00
joakin efb7b3d872 Wire up saving enabled/disabled in settings dialog
The save action has been implemented, and it is listened to by the
canonical enabled state in the previews reducer, and by the settings
reducer to perform UI changes.

The enabled state of the application has been kept in the preview
reducer as the canonical source of truth. See supporting changes for
documentation about the decision.

Actions:
* Introduce new action SETTINGS_CHANGE with the enabled status
* Trigger that action when clicking Save in the settings dialog

Reducers:
* Listen to SETTINGS_CHANGE in the preview reducer to update enabled
  status
* On the settings reducer
  * Handle the SETTINGS_CHANGE action
  * Add showHelp flag to determine if the help should be showing

Change listeners:
* Switch to compare past vs present changes in the implementation
* Handle showing and hiding the help

Supporting changes:
* On the rendered settings dialog:
  * Change #hide to actually just hide and remove legacy if statement
  * Add #toggleHelp method to show or hide the help dialog
* Add doc/adr/0003-keep-enabled-state-only-in-preview-reducer.md to
  support the decision of making the saveSettings action creator return
  a Redux.Thunk and keeping the enabled state just in the preview
  reducer.
  * Add NB to actions#saveSettings explaining and linking to the
    document

Follow commits soon:
* Persist the settings change to local storage when it changes, and
  unify with the preview change listener

Change-Id: I80dc5f29fbe6286f2e3e3b50d909894bc5041ccd
2016-12-14 14:35:59 +00:00
Sam Smith 9341685be3 Make API requests after 50 ms
I0b1cf333 introduced a regression where API requests were delayed by 500
ms rather 50.

Change-Id: I1bf953b29366b4ac66e7223c88484a192ba7e8d9
2016-12-14 06:51:28 +00:00
Sam Smith fcfe079d79 Hygiene: Organize reducers
Reducer changes:
* Break the mw.popups.reducers.rootReducer test into @@INIT tests
  for each reducer as there's shouldn't be any need to test that a
  framework works as documented.

Changes:
* Move the mw.popups.reducers#preview, #eventLogging, and #settings
  reducers into their own files in resources/ext.popups/reducers/.
* Make the associated tests mirror the new layout.
* Move the initialization of the mw.popups.reducers namespace into its
  own file.
* Remove the mw.popups.rootReducer property in favour of a simple
  private factory function per the reducer change above.

Change-Id: I94229d9ef1985f3806eec44c2e8234a5bbddc94f
2016-12-13 18:45:10 +00:00
joakin 68f04d62ec Wire settings dialog to show on SETTINGS_SHOW action
Now when on a hovercard and clicking the cog, it should show the hovercards
settings menu.

Additional changes:
  * Change the changeListener/settings to create and attach the DOM element
    just once to the DOM

Change-Id: Id685ddcda9532528fc62b383539788f785089ef0
2016-12-13 14:05:43 +01:00
jenkins-bot 492c170aae Merge "Introduce the settings dialog UI" into mpga 2016-12-13 12:54:37 +00:00
jenkins-bot f02957c762 Merge "Move settings template to ext.popups" into mpga 2016-12-13 11:39:36 +00:00
jenkins-bot f58942c3e8 Merge "Wire up showing settings via footer link or cog icon" into mpga 2016-12-13 11:27:26 +00:00
jenkins-bot f7746bb860 Merge "Introduce the settings change listener" into mpga 2016-12-13 11:26:48 +00:00
jenkins-bot cacd6ee04c Merge "Create the settings reducer" into mpga 2016-12-13 11:03:47 +00:00
jenkins-bot 4e27d4b5b3 Merge "Hygiene: Rename action constants for consistency" into mpga 2016-12-13 11:02:33 +00:00
jenkins-bot 1994c046a9 Merge "Hygiene: Use constant instead of string 'LINK_CLICK'" into mpga 2016-12-13 11:02:19 +00:00
jenkins-bot 82a61536d6 Merge "Move settings images and styles to ext.popups/" into mpga 2016-12-13 11:00:01 +00:00
joakin 4d5269320f Introduce the settings dialog UI
* Port ext.popups.desktop/ext.popups.settings.js to settingsDialog.js
  * Blank tests for now. Needs Qunit integration tests.
    * Transform into a factory function for future testing.
  * Saving functionality is commented out, will be removed when immplemented in
    the actions
* Add new incomplete action saveSettings
  * Will perform the saveSettings async tasks and then trigger enabling or
    disabling popups.
* Rename action settingsDialogClosed to hideSettings for consistency

Change-Id: I3936d3a4bc476de16d76025139be09f1798796c4
2016-12-13 11:42:21 +01:00
joakin 2f75b42271 Move settings template to ext.popups
Change-Id: I98ccb96f6662a4263245cee9d48faf976fbb154e
2016-12-13 11:33:18 +01:00
joakin e7c8f36a9f Wire up showing settings via footer link or cog icon
The changeListener is not wired up, so nothing will actually happen if you
click the link or cog. You should see redux actions flow trying to show it
though.

Change-Id: I29e629db63a4511a76c132f44f2ebf13254a4c6f
2016-12-13 11:32:57 +01:00
joakin 034f1840ab Introduce the settings change listener
Which for the moment is only in charge of showing and hiding the dialog

Change-Id: Ib57250236ff424abddb0fa627d2a48167a5d8d74
2016-12-13 11:32:53 +01:00
joakin 8415bd1e9b Create the settings reducer
Right now only with visibility state on it.

Change-Id: Idbe99aca652eb04357ba85f22ba413dcd38cd54b
2016-12-13 10:46:13 +01:00
joakin b33ba3d954 Hygiene: Rename action constants for consistency
No place for a single COG action, or verbs in past tense. Use NAME_ACTION like
in other actions.

Change-Id: I58b6756ce1e46eca10fe5ae03639b1dbcdc9907c
2016-12-13 10:46:13 +01:00
joakin 6444dcc7a6 Hygiene: Use constant instead of string 'LINK_CLICK'
Change-Id: Ifcd79b911cc3c3e3c2409ac2e2418128e3e7e11a
2016-12-13 10:46:13 +01:00
joakin ae22a00dc8 Move settings images and styles to ext.popups/
From ext.popups.desktop.

Change-Id: I55955d1506138e26d24fb9ebaf20ac3e168941ba
2016-12-13 10:45:51 +01:00
Sam Smith df441982fa {LINK,PREVIEW}_ABANDON_END logs an event
Since the user can dwell on a link, abandon it while moving to dwell on
the preview and vice versa:

* The time at which the link/preview was abandoned is recorded and can
  be updated.
* The event can only be queued when the link/preview has definitely been
  abandoned.

Reducer changes:
* Make the eventLogging reducer:
  * Calculate the time it took to show the preview.
  * Mark the interaction as finished when the user abandons the link or
    the preview.
  * Queue a "dismissed" event if the preview hasn't been shown or a
    "dwelledButAbandoned" event if it has when the abandon has
    finalized.

Bug: T152225
Change-Id: I6a254136f615484fc26e440fe5125289e74688a6
2016-12-12 19:00:33 +00:00
Sam Smith 60b65ad6c4 LINK_CLICK logs an "opened" event
Reducer changes:
* Make the eventLogging reducer queue an "opened" event with the
  requisite supporting data.

Changes:
* Update the Popups EventLogging schema to the latest revision, which
  defines the "opened" action.

Bug: T152225
Change-Id: I9d67daf83815f1c08b6479497c566dfa337de4bc
2016-12-12 18:59:57 +00:00
Sam Smith 5b76fb0276 LINK_DWELL starts an interaction
Action changes:
* Mix in timing information into actions that are likely to need it:
  LINK_DWELL, {LINK,PREVIEW}_ABANDON_START, LINK_CLICK, and
  PREVIEW_CLICK.
* Generate and include an unique token in the LINK_CLICK action.

Reducer changes:
* Make the eventLogging reducer:
  * Handle the LINK_CLICK action's new shape.
  * Start (create) and maintain a model of the user interacting with a
    link.

Bug: T152225
Change-Id: I671b12432ba2f7a93bf81043adb57ac30a4c38c3
2016-12-12 18:59:40 +00:00
Sam Smith 71b97cd089 Hygiene: Organise change listeners
Move all of the change listeners into
resources/ext.popups/changeListeners and remove the ChangeListener
suffix. Also, do the above for the associated tests.

Bug: T152225
Change-Id: I90ada465ea291d601f8f1c5c6e775148a2100319
2016-12-12 13:06:07 +00:00
Sam Smith bbeb618e1d Store/update user's preview count
Action changes:
* Add the PREVIEW_SHOW action.

Reducer changes:
* Increment the user's preview count in the eventLogging reducer.

Changes:
* Dispatch the PREVIEW_SHOW action when the preview has been shown.
* Add the previewCount change listener, which is responsible for
  persisting the user's preview count to storage when it changes.
* Call the change listener factories individually in
  #registerChangeListeners as their signatures aren't consistent.

Bug: T152225
Change-Id: Ifb493c5bff66712a25614ebb905251e43375420a
2016-12-12 13:05:50 +00:00
Sam Smith b7effdc000 Include user's preview count in BOOT action
Action changes:
* Include the user's preview count in the user property of the action.

Reducer changes:
* Make the eventLogging reducer add the bucketed user's preview count to
  the state tree.

Changes:
* Extract mw.popups.UserSettings#getPreviewCount and #setPreviewCount
  and associated tests from the ext.popups.core module.

Bug: T152225
Change-Id: I6b7afef31311be8fede685deb536f577845cb9cf
2016-12-12 13:01:44 +00:00
Sam Smith 4b74f72926 Include user's edit count in BOOT action
Action changes:
* Group user-related data in the user property, rather than repeating
  "isUser" and "user" prefixes.
* Include user's edit count in the user property of the action.

Reducer changes:
* Make the preview reducer handle the action's new shape.
* Make the eventLogging reducer add the bucketed user's edit count to
  the state tree.

Bug: T152225
Change-Id: I8fae9e2d0f6889ffdd30bb5192513d194f791967
2016-12-12 13:01:44 +00:00
Sam Smith 3f2752b039 Initial Popups logging implementation
Action changes:
* Include whether the user is logged in/out and information about the
  current page in the BOOT action.
* Add the EVENT_LOGGED action, which represents the eventLoggined change
  listener logging the queued event.

Reducer changes:
* Move all tokens and timing information from the preview reducer to the
  eventLogging reducer.
* Make the eventLogging reducer reset the state.

Changes:
* Add the mw.popups.createSchema function, which constructs an instance
  of the mw.eventLog.Schema object that can be used to log Popups
  events.
* Add the eventLogging change listener, which logs the queued event.
* Add hand-crafted, artisanal documentation.

Bug: T152225
Change-Id: I8a3f58358b211cc55417dcda7e796fe538e3d910
2016-12-12 13:01:44 +00:00
Jhernandez 2fa5b9ef02 Revert "Settings dialog"
This reverts commit 047bccfac1.

It created merge conflicts with a bunch of patches that were days old and about to be merged. Instead of rebasing 10 patches, let's revert this one, finish merging the other ones and just rebase and fix this one. I'll re-introduce the patch in a bit.

Change-Id: Ib495755b0ab4bfa5fdf5590b1271792862a47d4b
2016-12-12 12:54:33 +00:00
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 0bdce39bac Merge "FIXME: Document application initialization" into mpga 2016-12-09 10:26:04 +00:00
joakin ab4eff82e4 Don't use $.extend in nextState to preserve undefined fields
Since the state was cloned into an empty object with $.extend, undefined
properties were being deleted from it, not actually preserving those
keys. So:

         nextState({hi: undefined, ho: 1}, {ho: 2})
         //> {ho: 2}

It seems like a more consistent behavior would be to not lose any own
keys on the state as we do with the updates too, so that:

         nextState({hi: undefined, ho: 1}, {ho: 2})
         //> {hi: undefined, ho: 2}

Which is what this commit does by not using $.extend to clone the state
and instead just manually copy the keys to the new object, even the
undefined ones.

Change-Id: If4f2a3b0d25bb5ef34cfbc1f2c9c0b5479aeee9b
2016-12-05 20:16:36 +01: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
joakin 5dca90a7c3 FIXME: Document application initialization
Change-Id: Ib5de445b9074fc54a1158084fae480dd10640d6e
2016-12-02 12:14:02 +01: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
Sam Smith e80cc06e03 previews: More visual design tweaks
Per T150814#2833030.

Changes:
* Round the preview's border.
* Adjust the shadow cast by the preview.
* Make both fade-in and -out animations last 200 ms.

Bug: T150814
Change-Id: I55c728680ebb208e7cd1bd4c99a8453ae9915f2e
2016-12-01 09:59:55 +00:00
joakin 7cf3339f00 Hygiene: #nextState explain why we iterate keys to copy
It would seem that nextState should be $.extend({}, state, updates), but
as it was noted in the commit message that introduced the function
$.extend doesn't copy undefined|null values and that's why we iterate
manually over updates to copy any prop that exists over. That way we can
override properties to null|undefined in the state.

This commit expands the comment to explain so, and why OO.copy couldn't
be used.

Change-Id: If6c7119a4713328bb23c8f77042500510d515049
2016-11-30 17:02:00 +00:00
joakin e976af78e2 Hygiene: #processLinks Only use global variables at the edges
Instead of defaulting the config to the global mw.config when the param
is not defined, always pass it in (it is called just once in application
code), that way there's no need to test for the optional argument
behavior and the function is pure.

Change-Id: Ib1addb3060826f58dce2d6f928252ce1888a4293
2016-11-30 17:01:49 +00:00
joakin eb981bcd72 Hygiene: Fix docs reference to createUserSettings
Change-Id: Ia6e3301a03e69b5f052be0aa63052efbbccfe101
2016-11-30 13:50:18 +01:00
joakin 2ad2cf85ab Hygiene: Do not refer to specific UI elements in comments
Given this UI elements are going to change this comment would get
outdated really soon. Instead refer to generic interaction.

Change-Id: Icf56a864c47847bdef23985e9afd702ccb0de3b7
2016-11-30 13:50:17 +01:00
joakin 0bbaf0b7cc Fix UserSettings#getIsEnabled docs
Change-Id: I3fce221619857365dbf53903e6eecb108f3b45e6
2016-11-30 13:50:16 +01:00
joakin 550b68cab5 Hygiene: Correct return value type on mw.popups.wait
Change-Id: I0c5c5c1291f413241e855471b71b0e88f6de01c7
2016-11-30 13:50:13 +01:00
Sam Smith c284e910dd previews: Tidy up styles
I59ac2e32 removed support for non-SVG capable UAs but didn't remove all
of the associated styles.

Additional changes:
* Give the inner container a name, "mwe-popups-container", so that it
  can be styled with reduced specificity.

Supporting changes:
* Move the "core" styles into the ext.popups module.

Bug: T151054
Change-Id: I8deb6e76daf6f33fcb6f496129e6baf9e6793231
2016-11-29 18:11:11 +00:00
Sam Smith 3bf0d4a32a Make preview link to page
I14b437e7 introduced a regression where neither the preview's thumbnail
nor extract linked to the page.

Change-Id: I51793640d882aec711af8683ffbea794fad1b047
2016-11-29 12:59:53 +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 155a0fdf69 Render cleaner last modified timestamp
I14b437e7 introduced a regression where the `moment`ed last modified
timestamp wasn't rendered.

Additional changes:
* Remove the HTML comments from the preview template.

Change-Id: I78047dd0f8acc6292e0592887e128e6046119212
2016-11-28 15:05:31 +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 42a687e2f1 Visual design tweaks
Changes:
* Reduce the fade-in animation delay to 200 ms.
* Truncate a long extract by fading it out gradually.
* Increase the depth of the shadow cast by a preview and remove its
  border.

Bug: T150814
Change-Id: I2ec0c0472bc24767bbf1f4db000cc9d690454629
2016-11-25 08:02:45 +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 3d43263070 Contain Redux to one file
Move rootReducer into boot.js

Change-Id: Ia086fef2bd446b900d6b06e58ca9c998c2729a03
2016-11-16 12:00:33 -05: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 4324200e01 Add LINK_CLICK action
Change-Id: I2f3b5d51a68e4830abaf7f2dcd5d9ae0eb60d1e7
2016-11-10 12:05:38 +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
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
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 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
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
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
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
jenkins-bot 46274a5518 Merge "Regression: correctly log the click action total interaction time" 2016-10-04 10:23:46 +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