Page MenuHomePhabricator

Move MobileFrontend's VisualEditor styles back to MobileFrontend
Closed, ResolvedPublic

Description

See T140807#4526439.

These styles were moved to Minerva, but should have stayed in MF.

Related Objects

StatusAssignedTask
ResolvedDannyH
OpenNone
OpenNone
OpenNone
OpenNone
OpenJdlrobson
OpenNone
OpenNone
DeclinedNone
OpenNone
OpenNone
OpenNone
DuplicateNone
Openmarcella
Resolvedmatmarex
Resolvedmatmarex
Resolvedovasileva
ResolvedABorbaWMF
ResolvedNone

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2018, 10:17 AM

@Esanders Aye, yes, that makes sense!
I moved that file as part me helping Reading-Web finish their move of Minerva-specific styles from MobileFrontend to Minerva. I understood that Reading-Web prefers to keep these customisations centralised in the Minerva repository – which also allows them to re-use code by sharing a Less-variables import between them.
The main motivation behind this move was 1) Reading's goal of MobileFrontend not depending on Minerva (and vice-versa), and 2) Performance's goal of removing use of the deprecated hook that allowed that dependency to exist in an implicit way.
This file could certainly be moved back to MobileFrontend, in one of two ways: 1) Change the stylesheet to not use Minerva's shared import, or 2) By making the module in MobileFrontend explicitly depend on Minerva through a FileModule subclass that exists in Minerva. Whichever direction you choose, I'd recommend discussing that further with Reading-Web, as I can't decide this :)

TBH in the long term, I'd like to see all the VisualEditor Mobile code moved to VisualEditor and integrated with MobileFrontend. Likewise I'd like to see all the Echo code move to Echo. I think this is important as that's where your code lives and ideally MobileFrontend integration can happen inside one repo.
I think it's fine to move these skin styles to MobileFrontend if possible. I seem to recall there were issues adding styles here for some reason due to how skin/extension registration works but I forget what, so be careful if doing this, that the styles still apply.

marcella assigned this task to matmarex.Sep 13 2018, 2:51 PM
marcella moved this task from External and Administrivia to Up next on the VisualEditor board.
marcella added a project: Technical-Debt.

Change 460976 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] mobile.editor.ve: Simplify margin/padding for debug bar

https://gerrit.wikimedia.org/r/460976

Change 460977 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] mobile.editor.ve: Simplify left/right border of toolbar on >1000px screens

https://gerrit.wikimedia.org/r/460977

Change 460985 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Move Parsoid output overrides to 'mediawiki.skinning.content.parsoid' skinStyles

https://gerrit.wikimedia.org/r/460985

Change 460988 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/skins/MinervaNeue@master] Remove 'mobile.editor.ve' styles (move to MobileFrontend)

https://gerrit.wikimedia.org/r/460988

Change 460989 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] Add 'mobile.editor.ve' styles (move from Minerva)

https://gerrit.wikimedia.org/r/460989

This file could certainly be moved back to MobileFrontend, in one of two ways: 1) Change the stylesheet to not use Minerva's shared import, (…)

Done. My first two patches above remove uses of two of Minerva's variables.

I think it's fine to move these skin styles to MobileFrontend if possible. I seem to recall there were issues adding styles here for some reason due to how skin/extension registration works but I forget what, so be careful if doing this, that the styles still apply.

Minerva was adding the styles to the 'mobile.editor.ve' module from MobileFrontend using ResourceModuleSkinStyles; after my patch, MobileFrontend will add them directly when registering the module. This should have exactly the same effect.

Change 460976 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] mobile.editor.ve: Simplify margin/padding for debug bar

https://gerrit.wikimedia.org/r/460976

Change 460977 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] mobile.editor.ve: Simplify left/right border of toolbar on >1000px screens

https://gerrit.wikimedia.org/r/460977

Change 460985 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Move Parsoid output overrides to 'mediawiki.skinning.content.parsoid' skinStyles

https://gerrit.wikimedia.org/r/460985

Change 460988 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Remove 'mobile.editor.ve' styles (move to MobileFrontend)

https://gerrit.wikimedia.org/r/460988

Change 460989 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Add 'mobile.editor.ve' styles (move from Minerva)

https://gerrit.wikimedia.org/r/460989

Deskana closed this task as Resolved.Sep 27 2018, 1:30 PM