Commit graph

799 commits

Author SHA1 Message Date
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