Commit graph

593 commits

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