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
... 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
Changes:
- remove focus events listeners as they are triggered after switching tabs
- show PagePreview on keyup event
Bug: T158631
Change-Id: I7533f896604e0e0a8ea6e900ae4f7d12b6458836
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
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
... 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
Keep all configuration-like values in one file.
Changes:
- moved EXTRACT_LENGTH to constants.js file
Change-Id: Ibe5ecfc60f2c09a30a9ecb3586bc5fb6a7365476
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
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
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
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
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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
So that production code can be debugged properly. Independent source
maps files aren't loaded until the developer tools are open.
Change-Id: Ic8c3c10315d3a3be0f42505834005a3cab77d130
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
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
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
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
* 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
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
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
* 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
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
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
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
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
* 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
... 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
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
... 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
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
* 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
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
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
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
...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
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
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
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
Changes:
* Generalized previewCount change listener to syncUserSettings
* Added state.preview.enabled to be saved on change
Change-Id: I403a490fee9c8e125175996ba30c63c232b5598b
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
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
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
* 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
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
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
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
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
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
