Commit graph

16 commits

Author SHA1 Message Date
Jon Robson 2c09fd1d1c Generalize settings code (attempt 2)
This reverts commit a6a65204c6.
to restore custom preview types.

-- Changes since revert
The previous patch accidentally removed the syncUserSettings
changeListener. This has now been restored with several modifications:
* We have a migrate script which rewrites existing localStorage settings
to the new system
* The existing save functions are generalized.

The changes since this patch are captured in
Ia73467799a9b535f7a3cf7268727c9fab7af0d7e

-- More information

A new REGISTER_SETTING action replaces the BOOT action for
registering settings. This allows custom preview types to be
associated with a setting. They do this by adding the enabled
property to the module they provide to mw.popups.register

Every time the new action is called, we refresh the settings
dialog UI with the new settings.

Previously the settings dialog was hardcoded, but now it is generated
from the registered preview types by deriving associated messages
and checking they exist, so by default custom types will not show
up in the settings.

Benefits:
* This change empowers us to add a setting for Math previews to allow
them to be enabled or disabled.
* Allows us to separate references as its own module

Additional notes:
* The syncUserSettings.js changeListener is no longer needed as the logic
for this is handled inside the "userSettings" change listener in response to
the "settings" reducer which is responding to
SETTINGS_CHANGE and REGISTER_SETTING actions.

Upon merging:
* https://www.mediawiki.org/wiki/Extension:Popups#Extensibility will be
updated to detail how a setting can be registered.

Bug: T334261
Bug: T326692

Change-Id: Ie17d622870511ac9730fc9fa525698fc3aa0d5b6
2024-01-09 17:24:09 -08:00
Ed Sanders 883c8c1aca build: Update eslint-config-wikimedia to 0.22.1
Change-Id: I055517998ed06fccdf50ec31251ea6aa9040abb5
2022-02-28 13:18:24 +00:00
Sam Smith 8f5000f346 Remove Popups instrumentation
Bug: T267211
Change-Id: I640ab367cd235ab8da7dd70dbef7ae9076712e84
2021-07-29 17:01:01 +00:00
Thiemo Kreuz a35e35e3b3 Expand userSettings module for reference previews
Bug: T277639
Change-Id: I13acfb8bcc6e95fc28969072ec5420fd075d5096
2021-04-13 12:19:49 +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
Ed Sanders c74d2e5313 build: Update linters
Bug: T202739
Change-Id: Ie3f68977598f46f7e12b216f8381b2e9dc6d83ad
2018-09-11 10:26:35 -07:00
Stephen Niedzielski ae44042cbf Hygiene: add assertion messages
Change-Id: Ic0a47bd468532824e8648c3f6371cc403896603c
2018-05-08 15:55:23 -05:00
Stephen Niedzielski 57762e0417 Hygiene: favor const
Bug: T165036
Change-Id: I17d374eaac6627ca6a8ba178862b2a9cff2538c0
2018-03-21 10:44:24 +00:00
Stephen Niedzielski 0bee0906d4 Hygiene: replace var with let and const
eslint \
    --cache \
    --max-warnings 0 \
    --report-unused-disable-directives \
    --fix \
    src tests

Change-Id: I051275126ae7fa9affd16c2504017c0584f2d9c7
2018-03-20 14:14:02 -05:00
Stephen Niedzielski a67466acc0 Hygiene: replace obvious function methods
Replace easily identifiable object functions with method syntax:

  find \
    -not \( \( -name node_modules -o -name .git -o -name vendor -o -name doc -o -name resources \) -prune \) \
    -iname \*.js|
  xargs -rd\\n sed -ri 's%:\s*function\s*(\([^)]*\))%\1%g'

Bug: T165036
Change-Id: I90693ee99a6a36dff820dd5ae6f6000429763058
2018-03-20 09:27:07 -05:00
joakin 807100bcca Limit line length to 80 characters
Enforce it with eslint.

Ignore:
* Comment lines with eslint disable directives
* QUnit test lines as they contain long subjects (QUnit.* (only, test,
  module, skip, etc)
* Strings, since long strings are used extensively in tests
  * Ignore template literals for similar reasons
* Regex literals as they may be too long, but can't be easily
  split in several lines
* Long urls

See bug for more general proposal for eslint-wikimedia-config.

Bug: T185295
Change-Id: I3aacaf46e61a4d96547c513073e179ef997deb09
2018-01-19 14:20:39 +01:00
Piotr Miazga 910dd0408f if preview count stored in localStorage is not a number override it
There are some cases when the preview count stored in local storage
evaluates to NaN. When this happens we should override the value
to zero, store it in localStorage, and return it.

Bug: T168371
Change-Id: Ic44b7c7b5b716f6a0859f33278d56d2d95bbfb3e
2017-08-17 18:29:56 +02:00
joakin e6081106f1 Use EcmaScript modules instead of common.js modules
Why: Because they are the approved standard by TC39 and Ecma for
JavaScript modules.

Changes:
  * Wrap mw-node-qunit in run.js to register babel to transpile modules
    for node v6
  * Change all sources in src/ to use ES modules
    * Change constants.js to be able to run without
      jQuery.bracketedDevicePixelRatio given ES modules are hoisted to
      the top by spec so we can't patch globals before importing it
  * Change all tests in tests/node-qunit/ to use ES modules
  * Drop usage of mock-require given ES modules are easy to stub with
    sinon

Additional changes:
  * Rename tests/node-qunit/renderer.js to renderer.test.js to follow
    the convention of all the other files
  * Make npm run test:node run only .test.js test files so that it
    doesn't run the stubs.js or run.js file.

Bug: T171951
Change-Id: I17a0b76041d5e2fd18e2d54950d9d7c0db99a941
2017-07-31 23:05:44 +00:00
joakin 698a93f5bc Hygiene: QUnit setup -> beforeEach & teardown -> afterEach
The setup and teardown hooks on QUnit.module are deprecated on the
latest versions.

See https://api.qunitjs.com/QUnit/module

mw-node-qunit supports them but we shouldn't be using them.

Bug: T160406
Change-Id: I32c07f22d01d16449a6e37f46ff20c577a1f14c6
2017-04-26 12:34:12 +02:00
joakin 57854d4176 Tests: Migrate userSettings.test.js to node-qunit
Supporting changes:
* Use mw.Map stub
* Use assert.expect instead of number of assertions in QUnit.test
  (deprecated in newer QUnit)
* Don't specify assert.expect( 1 ), because it is the default (no
  assertions will make the test fail).

Change-Id: I64a3e76917e75b8c6d496f20e5b5dcafb338a46a
2017-03-01 12:47:07 +01:00