Commit graph

4 commits

Author SHA1 Message Date
Thiemo Kreuz c281bb9302 Clean up code enabling individual popup types
We still have 2 different mechanisms in place, maybe even 3:
* We simplify the CSS selector when we know a popup type is
  disabled, and it's impossible an anonymous user can enable it
  at run-time.
* We create that "initiallyEnabled" map that allows anonymous
  users to toggle the individual popup types at run-time.
* This map is also used to check if the footer link should be
  shown.
* There is also a wgPopupsReferencePreviews global that acts as
  a "kill switch". However, this is not a pure feature flag,
  but incorporates the user setting for registered users. This
  is currently partly redundant (checking
  `mw.user.options.get( 'popupsreferencepreviews' )` does the
  same) and can be removed later when the feature flag is not
  needed any more.

The footer link currently acts odd because anonymous users are
unable to enable ReferencePreviews, but get the footer link.

This patch introduces a 3-state model:
* `true` acts as before.
* `false` means a popup type is disabled, but anonymous users
  can enable it (i.e. this is the opt-out behavior for anonymous
  users).
* `null` means a popup type is not available at run-time, for
  nobody. Anonymous users can't do anything about this.
  Registered users must leave the page and change a setting.

Bug: T277640
Change-Id: Id8d1396c09cf0f706034a66f9cd3c880a8b33df8
2021-04-21 19:38:25 +02:00
Thiemo Kreuz 3466e66995 Minor cleanups to QUnit test setups
* That `this.user` is unused.
* Some tests missed a module name. This means they are reported
  as part of the previous module. While this is purely cosmetic,
  it's confusing to see the wrong module name in the report.

Change-Id: I73915d3c4fd9a03bda1ddc8dff6dd5539113c3cd
2021-04-16 13:23:10 +02:00
Thiemo Kreuz ddf574afa3 Remove not needed userSettings.hasIsEnabled()
Note how getIsEnabled() is documented: "if the user hasn't
previously enabled or disabled Page Previews […] then they
are treated as if they have enabled them."

In other words: The idea that the default should be true is
encoded twice in this code. This is just not necessary. We can
remove one without loosing anything.

Motivtion: Simplifying the code and reducing the package size.

Since the code fundamentally depends on this default value
anyway, we can clear the users localStorage when they decide
to go back to the default – instead of storing a "1" which
does the same as the default.

Change-Id: I2814a1e9269979918609162a508eeee6944d9e52
2021-04-08 11:57:52 +02:00
Thiemo Kreuz c5accc0300 Rename many functions and files for clarity
The main motivation here is to dramatically reduce the number
of places that use the same property name "enabled" for values
on different objects (e.g. "state", "actions", and "updates"
are all different things) with slightly different meanings. I
tried hard to come up with names that reflect better what each
meaning is.

Bug: T277639
Change-Id: Ie766259793f716262e3d4622ca55156d11f4842c
2021-04-08 11:04:02 +02:00
Renamed from tests/node-qunit/isEnabled.test.js (Browse further)