Commit graph

129 commits

Author SHA1 Message Date
Piotr Miazga 959bf40d9b Implement necessary wiring for preferences
Changes:
 - introduce unit tests
 - don't send module for anonymous users
 - renamed Module to Context
 - remove Config dependency from Popups.hooks.php

Bug: T146889
Change-Id: I3cbcdc1303411b28b613afa6dd1a00b410891471
2016-12-20 16:54:33 +01: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
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
jenkins-bot cacce7b03c Merge "Introduce Opt-In option on user preferences page" into mpga 2016-12-16 00:11:55 +00:00
Piotr Miazga f597e341af Introduce Opt-In option on user preferences page
This changes introduces new option on Special:Preferences page that
allows users to enable/disable Page Previews feature.
By default feature is disabled. Temporarily option in Preferences
page is smoke and mirrors as switching logic is still WIP.

Bug: T146889
Change-Id: Ifdd17ce265d2d4c7583433ed4991443c563f1fe3
2016-12-16 01:06:59 +01: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
jenkins-bot 1277baf54e Merge "Tag previews acceptance tests with @integration" into mpga 2016-12-14 11:54:12 +00:00
jenkins-bot 9ec1fd308e Merge "Hygiene: Remove RL-related step" into mpga 2016-12-14 11:53:13 +00:00
jenkins-bot 1d8a5e1e98 Merge "Hygiene: Remove "I am on the ... page" step" into mpga 2016-12-14 11:52:14 +00:00
jenkins-bot c47c1867bc Merge "Hygiene: Tidy up "core" acceptance tests" into mpga 2016-12-14 11:52:11 +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
Sam Smith 87ff0cae83 Tag previews acceptance tests with @integration
i.e. Allow the previews acceptance tests to be run against each Gerrit
change.

Change-Id: I17daaa955026c7fc5ef63b248c48932ecb75a1bc
2016-12-13 14:47:32 +00:00
Sam Smith 6a2f641cf4 Hygiene: Remove RL-related step
High-level acceptance tests shouldn't expose low-level concepts such as
the Resource Loader ("RL"). Merge the "And the RL module has loaded"
step into the "Given I am on the test page" step as they are only ever
used together.

Changes:
* Use the ArticlePage#wait_until_rl_module_ready helper method provided
  by mediawiki_selenium@1.7.3.
* Update and include the Gemfile.lock.

Change-Id: Ic81eae5dc0caa8ec51c91b21aef544f52d8c49e1
2016-12-13 14:46:03 +00:00
Sam Smith a0b01f86ab Hygiene: Remove "I am on the ... page" step
Merge the "Given the test page has been created" and "Given I am on the
... page" as they are only ever used together to mean "Given I am on the
test page".

Supporting changes:
* samples/links.wikitext -> fixtures/test_page.wikitext as it's more
  obvious what the file is.

Change-Id: Ide261a8e8c55133ab2a13a80c48e266252f61f5f
2016-12-13 14:25:37 +00:00
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
jenkins-bot 492c170aae Merge "Introduce the settings dialog UI" into mpga 2016-12-13 12:54:37 +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
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 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
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
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
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
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 8cd297c797 Hygiene: #processLinks Improve wording on tests
Change-Id: Ib6dd29d2c27dd10525fd9d875bfc860c5b76c841
2016-11-30 17:01:54 +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
Sam Smith e4719c4918 Don't hide preview if it's interacted with
Action creator changes:
* Make the linkAbandon action creator asynchronous by splitting it into
  two distinct actions, LINK_ABANDON_START and _END, the latter of which
  is dispatched after a 300 ms delay.
* Introduce the previewDwell and previewAbandon action creators. The
  latter is an asynchronous action that mirrors the linkAbandon action.

Reducer changes:
* Make the LINK_DWELL, LINK_ABANDON_END, and PREVIEW_ABANDON_END action
  hide a preview, if one has been shown.
* Make the LINK_ABANDON_END action NOOP if:
  * The user has interacted with another link, or
  * The user is interacting with the preview.

Supporting changes:
* Update the mw.popups.reducers#preview and #renderer unit tests to use
  an empty previous state so that the tests are more resilient to
  modifications of the state tree.

Change-Id: I2ecf575bbb59bb64772f75da9b5a29c071b46a8d
2016-11-28 17:15:37 +00:00
Sam Smith 1210f2f63e actions: Don't fetch if user isn't in condition
Change-Id: I6884f2da38c5b0addcd0a8b3325363efd46401f5
2016-11-25 21:22:09 +00:00
Sam Smith eabb7011fb Don't always render after the API request resolves
If the user abandons the link after the API request delay (500 ms) but
before the it resolves (~10e3 ms), then the preview shouldn't be
rendered.

Changes:
* actions: Include the link in the FETCH_START, FETCH_FAILED, and
  FETCH_END actions.
* reducers: If the active link has changed, then FETCH_END is a NOOP.

Supporting changes:
* reducers: Signal that a preview should be rendered and shown with
  preview.shouldShow.

Change-Id: I3dd1c0c566ec63de515174c14845d7927583ce93
2016-11-25 12:42:02 +00:00