| stjn | |
| Thu, May 9, 9:45 PM |
| F54243319: 2024-05-23_09-20-34 (1).gif | |
| Thu, May 23, 4:54 PM |
| F54243315: 2024-05-23_09-19-01 (1).gif | |
| Thu, May 23, 4:54 PM |
| F54243305: 2024-05-23_09-17-08 (1).gif | |
| Thu, May 23, 4:54 PM |
| F53961995: bqU4aXC.jpeg | |
| Mon, May 20, 3:06 PM |
| F53961981: ZGMk1Fq.jpeg | |
| Mon, May 20, 3:06 PM |
| F53961967: YxqNp7L.jpeg | |
| Mon, May 20, 3:06 PM |
| F53515545: Screenshot 2024-05-16 at 11.47.37 AM.png | |
| Thu, May 16, 6:50 PM |
| F53514793: image.png | |
| Thu, May 16, 6:40 PM |
Steps to replicate the issue (include links if applicable):
For logged-in users it is even worse (screenshot by another user):
What should have happened instead?:
Previously it was looking like this:
https://web.archive.org/web/20230709031443/https://ru.m.wikipedia.org/wiki/Coronatus
FlaggedRevs notices should be adapted to mobile again.
Whoever was responsible for this change needs to fix it back.
| AC | Status | Details |
|---|---|---|
| 1 | ✅ | T364587#9826730 pass per T364587#9827532 |
Seems like @Jdlrobson in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/FlaggedRevs/+/1020418 while fixing T361671. Merged by @Ladsgroup.
Making such an unwarranted (and unannounced!) breaking change while fixing a routine issue is really strange. (edit: better wording)
Change #1030598 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/FlaggedRevs@master] Fix display of flagged revisions overlay
Change #1030599 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/FlaggedRevs@master] On low resolutions, revert to simplified layout
Change #1030598 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] Fix display of flagged revisions overlay
Change #1030599 merged by jenkins-bot:
[mediawiki/extensions/FlaggedRevs@master] On low resolutions, revert to simplified layout
The work was done to remove old code and dependencies. The visual change wasn't planned and was a mistake on our behalf (I checked the desktop when I was reviewing, but not mobile). i.e. that wasn't the intention.
Does not LGTM:
https://ru.m.wikipedia.org/wiki/Сказочная_тайга
var(--border-color-base,#a2a9b1) matches to a colour with 2.37:1 contrast (fails WCAG A level)
parts of the FlaggedRevs interface are still not hidden like they were before
What do you mean? This is in scope: the colour that you added fails basic contrast requirements, part of the interface (– icon below the smaller notice) is not hidden even though it was before.
@stjn the colors for light and night mode were changed in T360231#9806643. I've commented there that this has a bug
parts of the FlaggedRevs interface are still not hidden like they were before
The display of this icon is intentional. The previous approach of maintaining different styles for different skins is not really sustainable given FlaggedRevisions maintenance state. It can be hidden in site/user styles if not desired/useful.
Why is displaying a hover-only element with no context in hover-less environment in any way better than not displaying it like it was before? I am baffled by this. OK, I will add the necessary code to hide it completely from Minerva (since I think the actual reason for this not being uniform with desktop is the fact that Minerva doesn’t load Common.css).
The real fix would be making it not hover-only, not hiding it – tapping it should also display the popup. By the way, I can simulate the hover by long-pressing it (Firefox on Android), although this is neither convenient nor intuitive.
Another problem a person reports with the new styles in https://ru.wikipedia.org/wiki/Википедия:Форум/Технический#Не_баг,_а_фича!
Actually, this means that Minerva places the FlaggedRevs notice in a different place from before. Previously it was just under the heading, now it’s under the buttons. @Jdlrobson can I ask you to take a look and/or clarify the difference?
@stjn can you provide some URLs for the above example and I'll take a look later today.
It's floated - looks like you have a local style here: https://ru.wikipedia.org/wiki/MediaWiki:Common.css#L-195
that needs to be copied to MediaWiki:Minerva.css ?
I've created T365257 for the hover behaviour (it's also not keyboard accessible)
Huh. First time I’m seeing that code. But I would argue that the screenshots above should still display correctly on the default FlaggedRevs setup. Even if some local overrides are missing (I will move them to https://ru.wikipedia.org/wiki/MediaWiki:Gadget-common-site.css now but I would argue this still needs a fix).
@Jdlrobson When you are logged in, you can scroll to the right and cut off a word as seen in the gifs below. Please review and let me know how to proceed, thanks!
Status: ✅ PASS
Environment: PROD
OS: iOS 17.1, Android 13, Android 14
Browser: Safari, Chrome, Firefox
Device: MBA
Emulated Device:BrowerStack
Test Artifact(s):
Visit https://ru.m.wikipedia.org/wiki/Хребет_Национальной_академии_наук_Таджикистана
✅ AC1: FlaggedRevs broken on Mobile
UPDATE: Pass per T364587#9827532
| Status | iPhone 15 Pro Max | Samsung Galaxy S23 Ultra | Google Pixel 8 Pro |
|---|---|---|---|
| Logged IN |  |  |  |
Thanks @GMikesell-WMF that's tracked under T365396 and out of scope for this ticket - this task was scoped only about the notice above the button.