I believe these additional newlines all make the code easier to read.
It's easier to see what belongs together, and what is a separate thing.
I found the Squiz.WhiteSpace.FunctionSpacing sniff very helpful to
enforce this code style. We enabled this already in almost all WMDE
codebases. It is not yet part of the upstream MediaWiki rule set, but
discussed.
Change-Id: Ibdf788529b28637bf98e7940c2516852c3afcef7
Minimize the difference between MobileFrontend and Popup's
configuration where possible. No build delta was generated so the change
is expected to be nonfunctional.
- Set the Webpack configuration file's ESLint environment to Node.js
instead of manually specifying Node.js globals.
- Directly export the configuration as a function instead of persisting
a local and assigning it to module.exports at the end.
- Export the configuration as a function and replace Node.js variables
with Shell compatible parameters. This required inlining a couple
single-use variables. See
I83e507fad1ee2f477bb95c2987d0b24f082b2165.
- Reorder configuration and update comments to match MobileFrontend.
Change-Id: I24545da2d029d08b0ea0e1330b6bb71ce423e6b7
The SVG masks used for previews are clip paths, making it hard to
visualize what their actual shape is. This patch extracts the paths
inside those clip paths into new SVG objects and renders them in
the Storybook app.
Also removes a few previews from the portrait/landscape thumbnails
so that the Knobs addon can function properly.
Change-Id: I160746d8877deaa12ace925ce170323946ce532d
The majority of this was fixed just recently via I8de42df. The sniff
makes sure all old and new code conforms to this style.
Change-Id: I8aecf3653d021fc8f86abcdc5939529864f86a48
-> Use single quotes for string literals.
-> Document missing parameter in test method.
This is just to improve on code readability, consistency
and maintainability.
Change-Id: I8de42df9f856ecb409637fe33b5f84b8bed1b547
* Explain the difference between the two hooks that both set config vars.
* Remove by-ref & that are not needed. This is a non-breaking change.
Even if the code calling a hook handler provides a variable by reference,
the hook handler being called does not need to require a reference.
Removing these & makes the code less confusing and easier to read.
* Replace an OutputPage type hint with a more narrow IContextSource.
That's all this code actually needs to know. It doesn't need access to
the entire OutputPage, so it doesn't need to require it.
* Update the signature of the ResourceLoaderGetConfigVars handler to
match the caller.
Bug: T215896
Change-Id: Ie1e4b50cd954812f71dd628003b8e9d40fdf5fe8
For popup types other than references this particular margin is calculated
as "2 lines + 7", which results in 47px.
The calculation implemented with this patch results in 55px. That's 8 more.
This also increases the total height of reference popups by the same amount.
Bug: T214169
Change-Id: Ie7870717d2fd2ce78268d1fc1b79d87eff059318
-> Objects by default as passed by reference so removed & in front
method header.
In addition, updated the PHPDoc comments to reflect the changes.
Change-Id: I69ad2090205f6bd694744779d2c46a3506c85378
As suggested in I69ad209.
As far as I can tell the idea was to be able to pass logger-specific
configuration to the logger factory, e.g. to be able to toggle certain
things the loggers would do. But this is not used at the moment. There
is not much value in keeping unused code around. It can esaily be
introduced again later when it turns out it is needed.
Furthermore, I'm told most of the logging functionality should be
removed anyway. See T193051.
Change-Id: I6b1ddb2a65eacc0e096f2ba44922d63e63212a65
Our documentation generator Doxygen is not able to understand @var tags
on constants. A constant is neither a "variable", nor is it possibly to
specify an allowed type for a constant. The type of a constant is strictly
derived from it's value. There is no way to change it, other than changing
the constant itself. There is not much value in repeating the type either.
If it's critical to know the type of a constant (which is a very rare
situation), that should be better explained in the comment.
Doxygen strips all @… tags it does not understand, and renders everything
else as a plain string. This makes it possible to use something like:
/**
* @const Description…
*/
But this is not different from:
/**
* Description…
*/
Change-Id: I4531d4f6b2ab2aca7a62f13f10a655f31c4d6a7a
Rename selectInitialGateway() to selectGatewayType() in
gateway/index.test.js. This test's import name was out of sync with the
declaration which made it more difficult to grep and reduced
consistency.
Change-Id: I60ae359043fdf76015d75b6438992ef6061a4d72
This happened one time before and was fixed via T212419. It now came
back after I2670331, a patch for T214861 that made the title parser
accept more links than before.
This patch implements a decision tree that goes as follows:
1. If it's a link to another page, it can't be anything but a page
preview, as having a peek into another page is effectively the
definition of a page preview.
2. Otherwise it's probably a reference preview, but only if:
* It's a link with a jump mark.
* The link is actually marked as being a "reference".
* Reference previews are enabled.
3. If neither of these definitions fit, do nothing. Note this is not
"hiding errors" we would be able to fix in this code. There are many
ways to manually arrange wikitext in a way that it looks like one or
the other, without fulfilling all requirements. Unfortunately the
user who would see error message or dysfunctional popups would not
be the same as the user who wrote such wikitext.
Bug: T216683
Change-Id: I8d021f19ddc73a261e6a0c62959ddd0cb1d3182d
- Drop unused moment, Redux, and ReduxThunk globals.
- Instead of using a confusing and deprecated boolean, specify that
globals are "readable" / "writable"
https://eslint.org/docs/user-guide/configuring#specifying-globals
Change-Id: I711a4ea7c51c9a560f20fc28bc99bd932c7e1e25
See the discussion at I6bd7acc. In this codebase it was decided to not
use "classic" JavaScript, but document it with the benefits TypeScript
comes with. Since I27b5cb0 a dev dependency is in place that makes the
upper case JQuery type work.
All the lower-case jQuery this patch happens to touch have been placed
by me, not knowing better at this point.
Change-Id: I76ef8eabaf4850f07b140dac6f489df37422263e
We run into some confusion with the previous patch Ifcc355f, and
decided to talk about the max-height for reference previews later. The
result of this conversation is this patch here.
The mocks and discussions in T214169 mention several numbers that all
conflict a little bit. In detail:
* We ignore the mentioned "4 lines", but go with 5. We assume the 4 is
a mistake. Using 5 is consistent with all mocks and all given pixel
measurements.
* The mock ask for a max-height of 106px for the container. With this
patch it's 100px, which is exactly 5 lines. The extra 6px don't do
anything as far as we can tell, but make the code confusing.
* The mock asks for 215px for the whole popup. With this patch it's
203px. Note this number is (intentionally) not hard-coded anywhere but a
result from all line heights, line counts, paddings and margins combined.
Bug: T214169
Change-Id: I18f61ed02ed506d48e3834e2ebc48b3392f7d732
These are annotations for ESLint as described at:
https://eslint.org/docs/user-guide/configuring#specifying-globals
I'm not sure where the `…: false` comes from. I assume it is a mistake
and does not have an effect.
I tried to move these annotations closer to the line they are about in
case there is only one line. And move it to the top when there is more
than one line using the global.
Change-Id: I4bd112c5fddd8a97d829a9b91707b8eb7cd7a332
