Commit graph

779 commits

Author SHA1 Message Date
Sam Smith 4db0043ee7 Hygiene: Tidy up "core" acceptance tests
Changes:
* Rename the somewhat awkwardly named "popups_core" Cucumber feature and
  its accompanying step definitions to "previews".
* Remove as many references to Popups and HoverCards as possible.
* Use language consistent with the QUnit unit and integration tests and
  documentation, e.g. the user dwells on a link.
* Remove settings-related step definitions from the previews step
  definitions.

Change-Id: Ibc0d07e8394aeb640f9efaabfe10be5317bf4b8c
2016-12-13 14:14:08 +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 1ea31cc003 Hygiene: Extract shared stub user object
Changes:
* Extract the various shapes of stub user object into the shared
  mw.popups.tests.stubs#createStubUser factory method.
* Register the ext.popups.tests.stubs module in
  PopupsHooks#onResourceLoaderTestModules and make the test suite depend
  on it.

Bug: T152225
Change-Id: I893b11461d8484bcaabfd950c2fa0dc672454a9d
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
Jeff Hobson fe0ed075a0 Make implicit dependency on mw.Uri explicit
Change-Id: Ica6b16fc3f246e02d7f3b446953cd51bd99e3595
2016-12-12 02:01:25 -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
jenkins-bot b5da2e1c13 Merge "Tests: Deeply extend prev state for test correctness" into mpga 2016-12-02 10:49:50 +00:00
Sam Smith e80cc06e03 previews: More visual design tweaks
Per T150814#2833030.

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

Bug: T150814
Change-Id: I55c728680ebb208e7cd1bd4c99a8453ae9915f2e
2016-12-01 09:59:55 +00:00
joakin ac264001f6 Tests: Deeply extend prev state for test correctness
As it is, that test is passing the same previous state and current state
because extend by default only copies one level deep, resulting in the
manual mutation mutating both prev and current state.

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

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

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

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

Change-Id: Ib1addb3060826f58dce2d6f928252ce1888a4293
2016-11-30 17:01:49 +00:00
joakin eae0a96e80 Hygiene: Only use that when necessary. Else use this
Change-Id: Idd91300041c1ae68a24a49626f79e97b3e8b2467
2016-11-30 17:01:35 +00:00
joakin cd7f74f691 Tests: Explain the second assert in the test
Like the next test also does, so that in case it fails we get some info
given the assert result doesn't match the test title.

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

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

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

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

Bug: T151054
Change-Id: I8deb6e76daf6f33fcb6f496129e6baf9e6793231
2016-11-29 18:11:11 +00:00