The text for 'type' in the documentation of the parent describes the
notification type can be 'all', following 'message' and 'alert'. That is
actually used, for instance inside of
mw.echo.Controller.markLocalNotificationsRead() function. The gap
between doc and implementation results in bugs. This resolves the gap.
Bug: T270879
Change-Id: I546aa42e927a05a5426db90153901ae632b97e36
This will cause them to be rasterized as the correct size
when used in emails. When used on the web (as SVGs) they have
their dimensions set by CSS.
Bug: T275936
Change-Id: Id7b76cf13bf9ebb88776d9c7eebc3944732ae743
== Problem 1 ==
As of I09c27a084100b223, tests/qunit/index.js or equiv was used to
load test files asynchronously from a using() callback. This was
untracked by RL or QUnit, and thus sometimes ended up finishing after
the test runner was already done executing all tests. In CI this
means the tests are sometimes never loaded and the browser (or Node)
process already killed before they even have a chance to arrive.
Prior to QUnit 2.17, this was no way of detecting this. As of
QUnit 2.17 (core upgraded last week) when running tests manually
the following helpful warnings appear in the console:
> [warning] Unexpected test after runEnd.
> [warning] This is unstable and will fail in QUnit 3.0.
> test @ qunit.js
> tests/qunit/model/test_mw.echo.dm.SeenTimeModel.js
> require
There were about 1072 instances of this warning, all from Echo.
Fix this problem by removing the async callbacks and specifying the
two modules as normal dependencies instead.
== Problem 2 ==
Class NotificationBadge was being loaded in a strange way out of
bound. This was a violation of module boundaries and should not be
needed other than for a temporary hack or other tech debt. More
generally when a test uses `packageFiles` this is a likely sign of
tech debt or misunderstandings.
Instead, depend on `ext.echo.mobile` and export/import the class
as normal.
After this, the test module can use `scripts` instead.
== Problem 3 ==
The `ext.echo.mobile` uses a Mustache template which the test
was also duplicating a reference to. This is no longer needed now.
Due to the `qunit/index.js` file carefully splitting the operations
between template assignment and file loading, I wondered whether
it was meaning to replace or mock it with something else, but it
simply refers to the same file and only does this because it wasn't
using the module directly. This is now resolved.
If you do need to mock in the future, this can simply be done
by assigning `NotificationBadge.prototype.template` from a
beforeEach() callback in the test suite, or by supporting it
property as a constructor option in NotificationBadge.js and
assigning `this.template` there, which is supported by the
mobile `View` class already it seems and would follow DI patterns
more effectively.
== Problem 4 ==
Most of the Echo tests were ignored sometimes and executed other
times.
The test for `ext.echo.mobile` in particular though was never
executed in CI specifically because:
> Undefined module: 'mobile.startup'
This became a hard error with this patch, which is fixed by
the CI config change with Ie9dabe3269c56fa76db8e51.
Bug: T299780
Change-Id: Ie4a87f3b8085fd6ae53ec586c1782cc266d5288a
It is common for internal files to export a single value, e.g.
when a file exports a single class or other special value.
However, this is uncommon for a module's overall export.
* It can create the misunderstanding that the init code is immediately
executed, when it is in fact delayed.
* This leads to the obscure `require()()` statement that is easy to
misunderstand.
* The least-effort way to expand this is by adding a statement
like `module.exports.Foo = Foo` after `module.exports = init`
which has the sublte behaviour that 1) it only works in this
order, not reversed as then Foo would be de-referenced by the
second assignment, and 2) it has the subtle effect of attaching
Foo to the `init` function as `init.Foo` which is non-obvious,
and 3) makes the init function unsafe to pass around, wrap,
stub or otherwise treat as a regular function.
Remedy by naming it as "init" on a regular module export object.
Change-Id: I51065e00f9dcaec075578a46df4de32c7a427df3
The .box-shadow() LESS mixin is now obsolete as box-shadow is supported
without any vendor prefix fallbacks, and planned to be removed by the
next MediaWiki release.
Bug: T283352
Change-Id: I3a4f5299b1d6b12e4dbfa68864613f5923957607
In Vector, these rules result in the anchor pointing to the wrong
place. They seem to be improve things in Minerva, so these are restricted
to that skin.
Bug: T276566
Change-Id: If871ab59fea519b48568781ffd9ddf6640006385
Note: This change would potentially affect all skins including modern
and legacy Vector and others.
The status quo when a talk notification is present is to transform the
talk link into an orange notification message. One option is to leave
the talk link and **add** a notification instead of transforming it.
This will result in two elements #pt-mytalk and #pt-talk-alert
This will interfere with styles targeting #pt-mytalk .mw-echo-alert
This impacts user styles but not anything in production.
Bug: T283811
Change-Id: I3e4be1381f9a2e9986b94b3b13df5ed64d09a59d
**Note**: This change will affect the order of the yellow talk page
message notification on legacy Vector/other skins by moving it from
after the `#pt-notifications-notice` element to before the
`#pt-notifications-alert` element. This was done because the
notification is related to the list of messages that appear when the
bell icon is clicked so having it in close proximity to that icon is
hopefully more intuitive than having it next to the unrelated inbox
icon. [1]
Per T274428, we need this notification to be inside the `notifications`
array instead of inside the `user-menu` array.
Additionally:
* Per T274428, update the notifications message copy to "You have a new
Talk page message"
* Remove the `onPersonalUrls` hook method inside EchoHooks,
unregister its use as a hook in extension.json, and update its
references in Echo.
[1] T274428#7113896
Bug: T274428
Change-Id: I5ae0ec089bcf0eec1ec7ac13f60e811f54e1d8e1
`list-style: none none` was a workaround for an IE 6 & 7 rendering issue.
Same goes for `list-style-image: none; list-style-type: none;` combination.
Let's unify to shorthand `list-style: none` for any non-ancient browser.
Change-Id: Ibdfb9a3cbd868d439504b59fa029e8da9a13f70a
Essentially: force the badges, when the personal menu is displayed
inline, to align correctly with the rest of the line instead of
hovering slightly above the line for no apparent reason.
This should fix nearly every other skin that doesn't already have
its own local fix, as 20px is already pretty well-centered with
normal-sized fonts when doing sane things with padding/margins/
line-heights. (Vector is not doing sane things with padding/
margins/line-heights in the personal menu, but hopefully forking
Vector went out of vogue ten years ago and people have quit doing
this by now, yes?)
This may also not be ideal for mobile skins. Breaks Timeless,
because I just completely gave up on trying to get this to behave
there and put it in its own little timeout box. Oops.
Change-Id: I73067bbc3c930ebf20aa492b53a4a5fe4259b607
These dont work. Timeless provides its own skinStyles for this module:
* Revert "Timeless skinStyles should be additive not replace"
This reverts commit 98d0d296a8.
* Revert "Use skinStyles for deciding where the text-indent should apply"
This reverts commit 8adab78547.
A new implementation in Vector means these are redundant:
* Revert "Item label can now use overflow hidden"
This reverts commit b5dd7baa08.
* Revert "Drop text indent in modern Vector"
This reverts commit 150fc7a16c.
Bug: T264339
Depends-On: I2afc12504d7184583fa8331479125474c68017dc
Change-Id: Ifd0dd5a0479588664b772fb839dbbcd8d7c47320
Since the icon is applied to the link element it is now possible
to apply overflow hidden to the label itself when its wrapped in
a span.
Bug: T264339
Change-Id: Ifdc1d152702c1f5338fd2969cccf07971d622fc7
It is documented that Uri can throw an exception so this should guard
against this case.
Bug: T261799
Change-Id: Ia45654194f853923a5b05ceff3ab2b47f6dd87d8
The code previously suggested it was only needed for Monobook and Vector
but it was also being used by timeless as discovered when 150fc7a16c
landed. This follow up applies the text-indent in the skin styles to ensure
it goes where needed.
Bug: T265806
Change-Id: I76a26a0a497f6bf7bb5f53967ea625f7e5d06ff7
In the new version of Vector the personal tools text is aligned right
rather than left. The text-indent is redundunt.
A background-position and center directive are added in preparation for
future changes we plan to the personal tools to make this code more
resilient.
Depends-On: Ic9df8c1cea0fef82461a84190689791ce2275812
Bug: T264339
Change-Id: I6bc80c8a42425599102370e5d90b48c07d360ccb
$.extend() with one parameter will merge the given options into the
global `jQuery` object, which was definitely not intended here. Maybe
the `config` variable was supposed to be passed as the second
parameter, but it doesn't look like that would do anything either.
Accessing the global `jQuery` object in that way ended up generating
completely unrelated warnings about deprecated `$.trimByteLength`,
which was reported in the task.
Follow-up to 7658e151d6.
Bug: T256732
Change-Id: I5f92b6725c8abdcfb676e6a09b610ea0dc8c245f
This issue was introduced in patch I41ad96b (T190397). The head
element is set to 44px height. However, the box-sizing is set
to border-box, which means these 44px include the 1px border at
the bottom. This leaves only 43px inside the head. The label is
meant to fit in there. Since the previous 44px don't fit, there
was always a scrollbar.
This could as well be fixed by changing the head to 45px.
However, this would make it different from the footer. I
believe both should have the same size for consistency.
Bug: T190397
Change-Id: Id0946d872a99fb8a1fbb33e6365fd78cd22eddd3
Calling overlay.hide() doesn't invoke the onBeforeExit handler
(anymore? not sure if it ever did), so we have to call this handler
ourselves when manually closing the overlay.
Bug: T258954
Change-Id: Ife5926241c0b8473607c14df0f89c794728566dd
Content font size should be 12.7px and this can be achieved
by just applying the 127% from globalWrapper.
This matches the font sizes on Special:Notifications.
Change-Id: I5369231da52343029ea2cdbfc790296c2257807a
Buttons should be positioned with margin, not padding,
as changing padding breaks hover and focus states.
In this case the position is already fine without
additional adjustment.
Change-Id: I8a7726b2e0988117084a1d0b060b4f649ba4cc75
