Page MenuHomePhabricator

Enhanced RC on Timeless with MF does not show expansion arrows
Open, Needs TriagePublicBUG REPORT

Description

I tested it on latest master versions of Timeless, MF and core and on 1.34.

Steps to Reproduce:
You will need a page with multiple edits made on it. Then, on Special:RecentChanges in enhanced mode these changes should be grouped and an arrow should appear to expand it.
The bug occurs when using mobile mode with the Timeless skin.

Actual Results:

image.png (405×897 px, 45 KB)

The arrows for expanding grouped RC items do not show up.

Expected Results:
The arrows should either appear normally, or RC items should not be grouped at all. The latter is what Minerva does.

I think the reason these arrows do not show up is that some RL module that provides them is not loaded. Maybe MF assumes these won't be needed as RC shouldn't be grouped, but it still is for some reason?

Event Timeline

It turns out Minerva disables enhanced RC altogether through a hook: https://gerrit.wikimedia.org/g/mediawiki/skins/MinervaNeue/+/0c488fa4f775815e96a64924c6b24ecd128a4b1b/includes/MinervaHooks.php#71

So it seems the code is (as always) badly split between MF and Minerva. I can think of these solutions:

  • We can move the aforementioned hook to MF, so that it would apply to any mobile skin.
  • We can also figure out which RL module we are missing when displaying Enhanced RC and try to load it anyway. I have a hunch this module may not target mobile at all.

@Isarra, would you mind taking a look? I can try to do the code wrangling, but an opinion from someone wise would be nice :)

I have a hunch this module may not target mobile at all.

That is exactly the case here.

I think targeting both mobile and desktop makes sense here, as Minerva disables enhanced RC in its own way, so there's no way this module could be loaded. Still, other mobile skins may wish to use enhanced RC.

On the other hand one could argue that grouping changes is not suited for mobile platforms, that is a valid argument, too. My view on this is this should be configurable by users. :)

In any case, we probably should do something about this.

I think targeting both mobile and desktop makes sense here

This. Even if minerva doesn't use it, any skin that does might as well have the module apply consistently, as it's not like it's going to break anything (more than is already broken).

Mind you, I'm of the general opinion that all modules should just default to targetting both unless they have separate modules for the same content (which they shouldn't, as weren't we trying to cut down on module count in general?). Even if the styles come out totally broken in one or the other, that just shows they need to be fixed/made responsive, or that there's some other general UX problem, which is something we should see! It's something our users should see, so they can investigate it and report on it and maybe even see if they can come up with their own solutions! They might be devs themselves, or devs in the making, and mw is all about people participating as a community...

Just randomly not having recentchanges or filepage or whatever styles at all is just... weird. Also makes responsively skinning these pages a pain in the arse because then we also don't necessarily have a module to attach our own actually responsive and compatible css to.