Commit graph

930 commits

Author SHA1 Message Date
James D. Forrester ca230b4c5d .nvmrc: Update now we're using Node 18, and make CI pass
* Babel upgraded to a version that works with new Node's OpenSSL.
* Various loaders upgraded to match
* Babel config changed as plugin-proposal-private-property-in-object is now official
* Webpack config updated as noEmitOnErrors was renamed to not be a negation
* Webpack config updated as namedModules was renamed to the wider moduleIds
* Ignore the built Webpack output from eslint

(cherry picked from commit 863c442c32)

----

Manually delete selenium tests, as they don't work and they're not worth the effort
of back-porting all the fixes to make them work.

Bug: T354943
Bug: T358102
Change-Id: Idcf164030bbc855a07acd4de45c57197ecc85095
(cherry picked from commit 57da341cf2)
2024-02-29 09:37:39 -05:00
Ed Sanders 883c8c1aca build: Update eslint-config-wikimedia to 0.22.1
Change-Id: I055517998ed06fccdf50ec31251ea6aa9040abb5
2022-02-28 13:18:24 +00:00
Volker E 830ec1b56b build: Update 'svgo' to latest v2.8.0 & re-crush images
Updating 'svgo' to v2.8.0 and newest Wikimedia SVG guidelines –
mainly around new whitespace features of SVGO.

Change-Id: I37da089916d2efad696989427b0a06b392d1d89c
2022-02-27 23:58:05 +00:00
Fomafix 0140767ea2 Simplify addClass with an array of classes
Since jQuery 3.3 .addClass() supports arrays. [1]

Replace
  .addClass( classes.join( ' ' ) )
by
  .addClass( classes )

[1] https://blog.jquery.com/2018/01/19/jquery-3-3-0-a-fragrant-bouquet-of-deprecations-and-is-that-a-new-feature/

Change-Id: Id5c0325971f3e9773155df7412ad0850cfaa11fd
2021-10-18 08:55:42 -07:00
jdlrobson fa19ee0d4f Adjust previews to meet specifications
Apply max-heights and tweak some of the margin values

Bug: T284643
Change-Id: I0c8a77e10c36ae444e541bb52a8797594c77db63
2021-08-25 22:55:26 +00:00
sahil 7142676629 selenium: Upgrade WebdriverIO to v7
Update npm packages: @wdio/* and webdriverio.

Bug: T274579
Change-Id: Ie9bd50e36f0c888a19e73ab9b424a8b6a5bd67fd
2021-08-24 17:29:16 +00:00
Timo Tijhof 06ab2d7676 build: Fix gaps in lint coverage and update old patterns
eslint:
* Remove hardcoded glob paths that are (relatively slow) to expand
  before linting could start, in favour of specifying directories
  which can be iterated while linting happens.
* JSON files in i18n/ were skipped, unlike other repos.
  JS code under .storybook/ was skipped.
  JS code under resources/ext.popups/ was skipped.
  Fix all these by doing what other repos do, which is to let
  ESLint iterate the repository and tell it which directories not to
  enter. This has the happy side-effect of making IDE integrations
  for ESLint work correctly (as customisation in package.json has the
  same problems as using Gruntfile, namely that invoking 'eslint'
  directly can't be aware of this), as well as allowing things like
  `eslint --fix` to be used.

```
/Popups/resources/ext.popups/index.js
  4:2  error  Unexpected var, use let or const instead  no-var
```

nyc:
* Remove SPAWN_WRAP_SHIM_ROOT.
  This was a hack for an early CI experiment that hasn't been
  needed for several years.

storybook:
* Remove gitignore entries for additional npm and build output in the
  storybook subdirectory, which appears to not exist anymore since
  189b386a13.

Doxygen:
* Sync with cookiecutter example from
  https://gerrit.wikimedia.org/g/mediawiki/tools/cookiecutter-library/+/HEAD/
  - remove redundant stuff,
  - enable quiet mode to hide verbose non-warning,
  - include CoC.

Change-Id: If8f6b833067192aea96a87f04c7978c9af11f996
2021-08-16 17:02:05 +00:00
Thiemo Kreuz a99bd13112 Rewrite linkTitle change listener for clarity/easier expansion
This was done while working on T277639 where we introduced
multiple "enabled" flags for individual popup types. This
change listener is one of very few places that work with
this flag. This patch is meant to make this code more robust
and easier to change.

A few unrelated but trivial changes are included that make
code shorter and hopefully easier to read.

Also fixes a bug with tooltips overlapping previews.

Bug: T287119
Change-Id: I7fb0a8d4bb9f5e78fe62cfca524cc157ea89a233
2021-08-11 18:33:34 +00:00
jdlrobson 75a9d81755 Fix pointer class behaviour
Add unit tests for landscape previews.

Bug: T287520
Change-Id: Ib9a7635b532248a4f44c308d7bb5dc7ab08667e6
2021-08-10 10:40:40 -07:00
jdlrobson a0754c8cf9 Storybook: Fix RTL
Provide containers for page previews examples

In storybook we avoid position absolute but this deviation from
how they behave in reality could lead to subtle differences that impact
the rendering of previews. It also doesn't allow to see the preview
in the context of the link which is an important part of visually
verifying the position of the pokey arrow.

This also allows us to rethink the broken RTL mode. We now scope
LTR rules to a LTR class that is present on the container, and use
the CSSJanus parsed stylesheet for RTL.

Change-Id: I189019824ddd6aa759790fd162ffcd543619a953
2021-08-06 14:40:10 +00:00
Thiemo Kreuz a01c31efd7 Show medium size (<320 wide, but >=250 high) images as well
The requirement for landscape images is to be at least 320px
wide. The requirement for portrait images is more relaxed,
only 250px high. Images that fall between these two
requirements currently don't show a thumbnail, even if they
could.

This change affects a very specific group of images:
* Square images from 250 x 250 to 319 x 319.
* Landscape images from 251 x 250 to 319 x 250.
* Landscape images from 319 x 250 to 319 x 318.

The most extreme ratio is 319 x 250. This will be cut to
203 x 250. I.e. the absolute extreme are 58px missing left
and right, but never more.

Requested at https://www.mediawiki.org/wiki/Topic:Vwl97pm6as9fuf6k

Additional stories for testing more extreme small images:
* Small Tall - 300x1000px
* Small Short - 300x200px

Update tests accordingly.

Bug: T268999
Change-Id: I811f1c0e7e9b0c30280b36a61cc7831a5b9e58c8
2021-08-03 13:17:06 -06:00
Sam Smith 8f5000f346 Remove Popups instrumentation
Bug: T267211
Change-Id: I640ab367cd235ab8da7dd70dbef7ae9076712e84
2021-07-29 17:01:01 +00:00
Thiemo Kreuz 5d2c5aeaa1 Remove a small piece of unused code from reference.js
Turns out this was only in place because the test was
(intentionally) incomplete. But it's dead code in
production. Let's get rid of it.

Change-Id: Ieeb145b6972dceb7eeda3a167d907b680d5c3ce4
2021-05-07 16:43:01 +02:00
Thiemo Kreuz 8a08a88f66 Remove/update some ESLint exclusions
Change-Id: Icd4c2d968e2b445a54709368de056373a7814919
2021-05-07 16:26:00 +02:00
Thiemo Kreuz e4da147efa Update JSDoc types and comments to be more specific
For example: We know the HTML element we are dealing with is
always an <a> element.

I believe the small changes in this patch are all obvious and
non-controversial. Please let me know if you disagree.

Change-Id: I9fe15845affffdd0f5f0fd6ef7d6b607cb567ac7
2021-05-07 11:57:58 +02:00
Thiemo Kreuz 92864de4d0 Tune confusing log error down to a warning
This is not really an error. It does not really have
consequences for the user, except:
* It might waste bandwidth because the module is loaded, but
  unused.
* It might waste a tiny bit of CPU time.
However, this can only happen for registered users, and only
with very specific combinations of settings (notably: a
conflicting gadget must be activated).

The reason it's marked as an error is that previously we could
give more guarantees about this. This changed recently. We will
check if the log message is still helpful and either remove it,
or restore the previous guarantees.

Bug: T271206
Change-Id: I218726c9c4879a405acef68710e79c6ac8d070fe
2021-05-06 10:08:29 +02:00
James D. Forrester 14c2102476 build: Upgrade eslint-config-wikimedia from 0.17.0 to 0.20.0
Some inline compatibility queries for the maintainers, where it
wasn't obviously intentional.

Change-Id: Idd659a0a0f7fb86c3f6870b8fe4608bbf8a45f05
2021-05-03 12:31:52 -07:00
James D. Forrester 2d5f39f4c7 build: Upgrade stylelint-config-wikimedia from 0.10.1 to 0.10.3
Also update coverage thresholds so the repo builds with newer
build tools.

Change-Id: I5561f2008b5e5462bb9afdb00c3ed86ff59c57f7
2021-05-03 12:29:30 -07:00
Thiemo Kreuz 75155a076a Fix settings dialog offering ReferencePreviews when unavailable
Open questions:
* It is possible to make the patch smaller in case this makes
  it easier to backport it. Is this worth it?
* This code is not covered by any test. I think it's worth
  writing tests. Most of this code will stay, even after beta.

Bug: T281352
Change-Id: I5f30054f1664643b427909f7fa189b4ea5e11879
2021-05-01 17:32:55 +02:00
Marcel Ruiz Forns a6a06f70bf Move VirtualPageViews back to mw.track
Use mw.track instead of dedicated tracker for VirtualPageViews.
This way we can migrate it to the new Event Platform client.
The new client does not observe DNT, which was the reason this
instrument was moved to a dedicated tracker on the first place.

Bug: T279382
Change-Id: I8bb515eab337ffed686ba7522bc6153cfdd8ca8d
2021-04-29 16:15:16 +02:00
Thiemo Kreuz 275a67c730 Make checkboxes use upstream mediawiki.ui.… styles
This does not fully solve the ticket, as these are not the
actual OOUI styles. But it's already much better than the
unstyled checkboxes before.

Bug: T281227
Change-Id: I9a5023482774c09aa73845ca6dfd1c4926f088e1
2021-04-28 10:37:07 +02:00
jenkins-bot 23c09a5d2a Merge "build: Upgrade 'svgo' & re-crush SVGs" 2021-04-28 07:51:03 +00:00
Jan Drewniak b6fceb2fda build: Upgrade 'svgo' & re-crush SVGs
'svgo' upgraded to v2.3.0

This includes:
- Replacing .svgo.json with .svgo.config.js
- Updating the SVGs files. This amounts to changes in the order of
  some attributes.
- Adding the minify-svg command as part of the npm run test command

Bug: T278656
Change-Id: Ia38332be68b8ac47a31caf30272920c0f0c12053
2021-04-28 07:28:16 +00:00
Thiemo Kreuz 3a00223a36 Separate reference preview settings in beta & non-beta
This is the smallest possible patch for a backport.

While it seems there is zero technical reason to limit user
option keys to lowercase with dashes, it's best-practice.
Same as message keys.

Bug: T281235
Change-Id: Ia4a45cf4459543c81b23b757ae9c2cfaf9676894
2021-04-27 16:15:17 +02:00
Svantje Lilienthal 3b6d9ccc9d Renamed settings local from simple to page
We changed the handling of the popup settings. They now contain popup types (pages and references). 
To not get confused with the old setting options (simple, advanced and off), 
we would like to rename the local for the simple setting to page. 
It should already contain the correct string.

Bug: T277639
Change-Id: I8847b890e9e31602277a92d82a188fcdd3eea855
2021-04-23 14:35:06 +00:00
Thiemo Kreuz d0872e3025 Clean up popup type settings related code
* Change more places to not hard-code the popup types, but use
  loops and such.
* Change many `function ()` headers to use the more streamlined
  ES6 sytnax.

Bug: T277639
Bug: T277640
Change-Id: Ifece87d51012e0e069286453b27f5c9ae273710e
2021-04-23 15:38:38 +02:00
Thiemo Kreuz a2c8417d74 Add test case for syncUserSettings with reference previews
Code was introduced in I13acfb8, but not individually tested
so far.

Bug: T277639
Change-Id: Ica4527d3514246069cce7f5848c22764c6306531
2021-04-23 13:05:09 +02:00
Svantje Lilienthal e5909e06c5 Updated settings link for anonymous users
Bug: T277640
Change-Id: I5abfed7ba1d55286276d03141dd3af33008b7e04
2021-04-23 12:46:31 +02:00
Svantje Lilienthal 09c2c52945 Added popup types handling
We added reference preview as a checkbox the the
anonymous user settings. To handle both popup types
(pages and references), we changed the usage of
preview.enabled. We pass on all types as a map
inside preview.enabled. The footer link to edit the
settings will appear for anonymous users if at least
one type is disabled.

Bug: T277639
Change-Id: I860a1b35ac7749d8d0884575f6acb7186ad8e4d0
2021-04-23 12:14:23 +02:00
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
Svantje Lilienthal cf9258b0c5 Changed radio button to checkbox in anonymous user settings window
Bug: T277639
Change-Id: Ifa84dda88a8a167fc8ed2cef17254c1b722ade2d
2021-04-21 14:13:47 +02:00
Thiemo Kreuz e19b557227 Merge duplicate ReferencePreviews Beta feature flag
Change-Id: Ia7f1c9128460bc5a139e37eb16fff649e80cd20f
2021-04-21 10:46:05 +02:00
WMDE-Fisch 190962cc5b Remove dead code around cog wheel dialogue
An "advanced" option was first introduced in 2014 via patch I374805e
(originally named "monitor-or-edit", renamed via patch I7b4f6d2).

The isNavPopupsEnabled() function was added in 2016 via patch
Ic660f48.

The code that disables the extension entirely the moment the NavPopups
gadget is enabled was added in 2017 via patch Ia474b1b (T151058) and
patch Ia837816 (T160081).

As of now, the "advanced" option can only be seen in an extreme edge
case:
* Only for anonymous users.
* Only if NavPopups is enabled by default for anonymous users.
* Only if the $wgPopupsConflictingNavPopupsGadgetName setting is
  misconfigured.
* … or if NavPopups is not a gadget in the first place, but e.g.
  loaded via Common.js.

In this situation the settings dialog opens with all *3* options. This
is broken for several reasons:
* The "simple" option enables the extension, but doesn't disable
  NavPopups. Both trigger, resulting in both popups being displayed
  the same time.
* Since "simple" is the default, this bogus behavior is the default
  for anonymous users.
* The "off" option doesn't stick. Every time the settings dialog opens
  "advanced" is checked instead.
* "Off" can't work anyway. There is no code to disable the gadget.
* Only the "advanced" option "works", but more by accident.

It's unclear how to fix this:
* There is no code that does anything with the "advanced" option. It's
  not even stored. The behavior of the option is identical to "off".
* The code appears as if "advanced" was meant to be shown instead of
  "off". I.e. anonymous users can only choose one of the popups, but
  not disable both. But there is no code to hide the "off" option.
* The bug when both popups are displayed was fixed in 2017 via an
  entirely different mechanism. Re-introducing "advanced" does not
  only mean duplication, it's unclear how the 2 mechanisms are meant
  to work together.

It really, really feels like this was just forgotten.

Bug: T278949
Change-Id: Iab21f3a649a5b2f19ebb0d0dbb45ce1450c65678
2021-04-15 15:29:16 +02:00
Andrew Kostka 398565b1d6 Track anonymous enables/disables of reference previews
Bug: T277641
Change-Id: I89ee3ff280e79f560bf613a110a1c9e0b3ba6648
2021-04-15 14:21:32 +02:00
Thiemo Kreuz 622fb6c5e8 Allow to disable/enable popup types individually
Bug: T277639
Change-Id: I0742b8ab1c5ed0b374d4f9447bebc46a35207339
2021-04-15 11:30:36 +02:00
Thiemo Kreuz 4b1f020b9d Avoid hard to read jQuery.extend() in tests
… as well as in one place in production code. The motivation
for this change is to make the code easier and faster to read.
There is a little bit of duplication in the test setup now.
But I would like to argue this is a good thing. The values are
rather trivial. The difference (or absense of a difference) is
much easier to see now.

Change-Id: I9aa95b59f0c45ea7c9257970e2fcdba3a000d234
2021-04-15 10:47:44 +02:00
Thiemo Kreuz b28f48d6d6 Rename variables in change listeners for clarity
This patch does nothing but rename a pair of variables:
"prevState/state" becomes "oldState/newState". Reasoning:

1. The abbreviated "prev" is confusing, especially because we
are in a codebase that is all about "previews".

2. We are in a context that is all about a state **change**.
Change listeners get notified about the change from one state
to another. While it would be possible to stick to the already
mentioned "previous/current" terminology, I find the word
"current" confusing. What is "current" in this context? Did
the state already change? Am I notified about a change that is
**going** to happen or already happened? Is this even relevant?
I don't think it is. Therefor "old/new".

Another possibility is "previous/next".

Change-Id: Id886e1a095967fe86fb9021f59e335c62da8994e
2021-04-15 10:19:40 +02: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 51dd4b2807 Make nextState support non-flat updates
The nextState() function was not able to understand updates that
are deeper than a single level. Example:

nextState( state, { pagePreviews: { enabled: true } } )

Before, this would replace whatever was in the "pagePreviews"
property with { enabled: true }, but not merge it. In some places
a single level of recursion was done manually because of this.
This can be removed now.

This is done in preparation for splitting the "enabled" flag into
separate ones for each popup type.

Bug: T277639
Change-Id: I35911c18018ba7cd1633a4c882b978656c3fee36
2021-04-12 13:46:18 +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
Thiemo Kreuz 1f052e0dcb Make syncUserSettings support dot-separated paths
… instead of only 2 hard-coded nesting levels, as it was before.

This is done in preparation for splitting the "enabled" flag into
separate ones for each popup type. This patch here doesn't change
any behavior or internal representation of the states.

Bug: T277639
Change-Id: Icad669d1c9675ad6de22f478e254debe5d1936d7
2021-04-06 17:46:21 +02:00
Clare Ming 4908f82c01 Add title attribute to settings gear icon
Add message, description, extension for title. Update createPagePreview, renderPagePreview methods to add title attribute to settings gear icon. Add test for title attribute. Increase maxSize, maxAssetSize, maxEntrypointSize. Add compiled js files.

Bug: T274887
Change-Id: Ibb29deb3418569d8283b954b4b22074423e78bda
2021-04-02 13:30:07 -06:00
Noam Rosenthal e4ee4ed5cf Set thumbnail width to 215px
When it was min-width: 215px, it could sometime
grow and break the popup's layout.

Bug: T276668
Change-Id: If88887920c48c0a7f6530d1a7600f25fd2a9579b
2021-03-22 14:40:27 -07:00
WMDE-Fisch 58f820009e Use flags to represent settings
To reduce size of code added to the <head> and increase performance.
The increased bundlesize is still less than the size spared bytes in
ResourceLoaderGetConfigVars. - But nevertheless the main gain is loading
less in the <head> anyways.

To avoid further complexity in the code, the bitmask is converted to
the according config setting early on instead of adding checks on the
bitmask all over the place.

Tests will be added in follow ups.

Bug: T276716
Change-Id: Ib4f82bed58295b25f0a41cb37e36244e45f16317
2021-03-11 13:38:06 +01:00
Andrew Kostka 9fcdaf5c00 Fix maximum height of reference popups when in beta
This fixes a small regression introduced in Ia61f1b7. When
reference previews are still enabled as a beta feature the
unnecessary footer is hidden, therefore, we need to account
for this when calculating the dialog's height.

Bug: T234205
Change-Id: I1c142019031ab954550e237ddb23824da1aee8db
2021-03-05 12:52:13 +00:00
Svantje Lilienthal fb0863f101 remove spacing, when the footer is empty
Bug: T276200
Change-Id: Id5d30a53251bdf3674388cbdb29d93a0d52b3266
2021-03-04 16:22:55 +00:00
Andrew Kostka 82d54945e4 Combine page and reference preview preferences
Update copy and remove unnecessary reference preview preference
in favor of using the default preference. It seems there is no
stable method to link to the subsections on the preference page
for gadgets. So in all cases does the link just point to the
gadgets pref page.

These changes should only be visible when reference previews
are no longer marked as a beta feature.

Bug: T265709
Change-Id: I7b8ab91331092ada04b230315373548673b9272c
2021-02-25 10:21:16 +00:00
Thiemo Kreuz 513899d808 Much more relaxed reference type detection
* When there are multiple <cite> elements, the first that contains a
  known class is used. We assume the earlier one must be the relevant
  one.
* When there are multiple known classes in e.g. <cite class="web news">,
  the last one is used. This follows how CSS works.
* There is extra validation if the corresponding message exists, just
  to be sure. This is cheap.

Bug: T274979
Change-Id: Iee3481ea16af96b40faf978e254718e5a48917f3
2021-02-22 11:55:03 +01:00
Svantje Lilienthal 1962372a44 Add cogwheel to disable ReferencePreviews
I added the common styling for the setting icon to the popup.less
and removed the now empty pagePreview.less.

Bug: T234205
Change-Id: I2a82831bc71a4208c4b66c18e2a4613127c43e1f
2021-02-16 15:39:43 +01:00