Remove rarely used 'mediawiki.ui.anchor' module.
In use by
Suggestion to move the styles to Flow and leave them in MF/MN?!
And after that, remove the module.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Open | Jdlrobson | T182050 Remove all mediawiki.ui components and libraries in favor of Codex | |||
| Resolved | Jdlrobson | T235961 Remove 'mediawiki.ui.anchor' module |
Change 609272 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/Flow@master] Add 'anchors.less'
@Volker_E If re-use of the functions is considered useful, those could be moved more or less for free to mediawiki.mixins (possibly still as their own file if you prefer). Because mixins don't add anything to the output by default. Basically all of anchors.less except the two lines that define .mw-ui-anchor could be kept in that case and included by the two callers.
No problem either way :)
@Krinkle For the code, a bigger conversation would be if MW core plans/needs to carry UI classes and we should “only” deprecate and remove the mediawiki.ui components public API.
Thinking if continuing any mw-ui prefix continues to make sense versus mw only. Although everything that ends up as a CSS class is here in one way or the other for the user-interface anyways. So mw on its own should be sufficient.
For the specific case, I don't see a lot of use case for things like
.mixin-mw-ui-anchor-styles currently or in planned future. If there needs to be such a mixin, I'd rather make it part of a better architecture and move it to another place like 'mediawiki.skinning'. See also T212095.
'mediawiki.ui' is dead and should be removed with a few things carved out into better places.
So the suggestion is to avoid code duplication by using a mixin like https://github.com/wikimedia/mediawiki/blob/master/resources/src/mediawiki.less/mediawiki.ui/mixins.buttons.less ?
I am a little troubled by having that class used in 2 repos with 2 different implementations. Remember Flow and MobileFrontend code is loaded at the same time, so we'd also be double loading these anchor styles on every Flow board. How many bytes are we talking? (I suspect not many but we should document it). Could we change the class names as part of this change?
@Volker_E I have no good answers there other than that it seems like a simpler and more minimal change to keep this task scoped to phasing out the RL module only, Thus effectively still re-using that code.
If the look of those styles is wrong and if that shoukd be done differently or based on other concepts instead for Flow and/or MF, that seems worth doing either as a separate change later (and then remove the mixins) or as a separate first (and then just remove the module outright).
The only issue I've discovered in Flow/MF is, that MF is replicating mw-ui-anchor rules. The only product using these remains Flow, which is supposed to be phased out. Remember, every page loads the mediawiki.ui modules right now. So MF would actually profit from this change with removing the whole 'mw-ui-anchor' module in my understanding.
Remember, every page loads the mediawiki.ui modules right now.
Not following this. MobileFrontend loads mediawiki.anchor. module via JavaScript to style https://github.com/wikimedia/mediawiki-extensions-MobileFrontend/blob/master/src/mobile.startup/Anchor.js. It should continue to do so after this change.
Pretty widely. Not gonna happen in time for 1.35. I think we're going to be living with the CSS for some time, so to me the mixin approach makes sense for now, but it's also fine not to use it, if you are okay with maintaining 3 different identical stylesheets, 1 of which is in a still active extension.
Best course of action (and least path of resistance) here to get this removed in time for 1.36 is:
Change 609272 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Add 'anchors.less'
Okay, if this is the approach we're taking please at the very least rename the class in MobileFrontend by a change to Anchor.js as part of these changes :) !
@Jdlrobson This would be important to get a clearer picture.
Also, mediawiki.ui is deprecated for a long while already. With that, mediawiki.ui/anchor is too.
From a code perspective the mediawiki.ui resource loader module is hard deprecated but the mediawiki.ui.anchor on the other hand is completely separate and like mediawiki.ui.button has seen no formal deprecation has occurred (yet). Deprecations don't apply just because they are similar named things. Therefore you can't remove mediawiki.ui.anchor you can only deprecate it per the deprecation process (add a deprecated field to its RsourceLoaderModule).
To deprecate it, we would need to copy the styles to MobileFrontend from core (like you did in Flow) but also rename the class (to avoid clashes) per T235961#6283947. That in itself is a lot of work to do in the space of a week.
It's too much work right now to reconsider it entirely - the Anchor component is used in far too many places (MobileFrontend, Minerva) to be completely reworked in that timeframe.
@Jdlrobson I come from a fundamentally different understanding of deprecating 'mediawiki.ui' module. This is the parent, with it all children are deprecated automatically.
It would be helpful to have a clearer picture where and if mediawiki.ui/anchor is even in use in MF? Like I've said above, I just see it in production in Flow views…?
I suggest dropping this back to backlog to keep us focused on the search feature for now.
CodeSearch isn't showing a lot:
https://codesearch.wmcloud.org/search/?q=mw-ui-anchor&i=nope&files=&excludeFiles=&repos=
anchorClass is only used on SpecialMobileEditWatchlist.php https://codesearch.wmcloud.org/search/?q=anchorClass&i=nope&files=&excludeFiles=&repos=
I don't think it's worth retaining the module and this class for one use case.
anchorClass is only used on SpecialMobileEditWatchlist.php https://codesearch.wmcloud.org/search/?q=anchorClass&i=nope&files=&excludeFiles=&repos=
I don't think it's worth retaining the module and this class for one use case.
That's the only usage in PHP, but in JS it's use a lot more:
https://codesearch.wmcloud.org/search/?q=Anchor%5C%27%20%5C)&i=nope&files=&excludeFiles=&repos=
That aside, I don't think usage has ever been a question here. My understanding is we got halfway through deprecating this and got distracted by other things, but yes we should remove it from core.
To deprecate it, we first need to copy the styles to MobileFrontend from core (like we already did in Flow) but also rename the class (to avoid clashes with Flow) per T235961#6283947. Porting MobileFrontend to Vue.js would result in the same outcome.
@Volker_E have you got bandwidth over the next month to help me move this module into Minerva and deprecate the core module?
Change 932461 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Remove mediawiki.anchor from MobileFrontend, use Codex link
Change 932462 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/extensions/Flow@master] Remove mediawiki.ui.anchor dependency
Change 932463 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Remove mediawiki.ui.anchor dependency
Change 932464 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):
[mediawiki/core@master] Remove mediawiki.ui.anchor
Change 932462 merged by jenkins-bot:
[mediawiki/extensions/Flow@master] Remove mediawiki.ui.anchor dependency
Change 932461 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove mediawiki.anchor from MobileFrontend, use Codex link
Change 932463 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove mediawiki.ui.anchor dependency
Change 932464 merged by jenkins-bot:
[mediawiki/core@master] Remove mediawiki.ui.anchor