Page MenuHomePhabricator

Parsoid style should target mobile as well as desktop
Closed, ResolvedPublic

Description

as a logged in user with alpha mode set, attempt to edit a page.

the VE controls never appear, user sees only a spinner forever.


Version: unspecified
Severity: normal

Details

Reference
bz64607

Event Timeline

bzimport raised the priority of this task from to Unbreak Now!.Nov 22 2014, 3:16 AM
bzimport set Reference to bz64607.

bingle-admin wrote:

Prioritization and scheduling of this bug is tracked on Trello card https://trello.com/c/lTCoRTO5

no fatal errors appear in the logs on beta labs

There is a JavaScript error

Error: Unknown dependency: ext.parsoid.styles
Error {stack: (...), message: "Unknown dependency: ext.parsoid.styles"}

Looks like ext.parsoid.styles needs to be set with target mobile.

Change 130388 had a related patch set uploaded by Jforrester:
Load Parsoid style module for mobile as well as desktop

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

Change 130388 merged by jenkins-bot:
Load Parsoid style module for mobile as well as desktop

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

It's not entirely clear to me why Parsoid needs its own styles...
http://git.wikimedia.org/blob/mediawiki%2Fextensions%2FParsoid/770ec8594626e0d8d8851adb4a2520ff34f18f24/php%2Fmodules%2Fparsoid.styles.css

Shouldn't this be decided by the skin..?

(In reply to Jon from comment #6)

It's not entirely clear to me why Parsoid needs its own styles...
http://git.wikimedia.org/blob/mediawiki%2Fextensions%2FParsoid/
770ec8594626e0d8d8851adb4a2520ff34f18f24/php%2Fmodules%2Fparsoid.styles.css
Shouldn't this be decided by the skin..?

These are replacing the styles inside MediaWiki; long-term this module will be moved into MediaWiki core as part of the skin refactor.

For now the goal is to have a central place to develop Parsoid-specific content style tweaks that can be used by all Parsoid users (VE, Flow, Content Translation, offline tools etc).

This actually causes a CSS regression on mobile.

These Parsoid styles slightly impact the look and feel for references in the Minerva skin after VE has been loaded. These styles shrink them, making the touch area smaller and virtually unclickable.

Why not put it straight into core or even better specific to Vector skin? All these moving parts make things super complicated.

Also James, please link me to what "the skin refactor" is. It's not clear to me what you are referring to...

(In reply to Jon from comment #9)

Also James, please link me to what "the skin refactor" is. It's not clear to
me what you are referring to...

No idea; I'm not doing it. I'm just fixing things.

(In reply to Jon from comment #9)

This actually causes a CSS regression on mobile.
These Parsoid styles slightly impact the look and feel for references in the
Minerva skin after VE has been loaded. These styles shrink them, making the
touch area smaller and virtually unclickable.

We might need to add media queries to handle mobile-specific styling for Parsoid-specific content. Could you add a patch for this?

Why not put it straight into core or even better specific to Vector skin?
All these moving parts make things super complicated.

I'm not opposed to this, but there is still some value in developing the Parsoid styles somewhat independently for a while. Folks are hesitant to merge styles that are not yet used for normal page views, but that will change with broader Parsoid content use.

We might need to add media queries to handle mobile-specific styling for
Parsoid-specific content. Could you add a patch for this?

A better approach might be to simply wrap them in a media query that targets larger screens (e.g. take a mobile first approach) - this would solve the issue with mobile as the existing styles will kick in - sadly however this will mean the rules are loaded unnecessarily on mobile.

My main worry is this file getting even more bloaty. Would it not make sense to use skinStyles on this module so that the styles only apply to Vector/Monobook?

I'm not opposed to this, but there is still some value in developing the
Parsoid styles somewhat independently for a while. Folks are hesitant to
merge styles that are not yet used for normal page views, but that will
change with broader Parsoid content use.

Yeh I'm aware of this anti-pattern in the MediaWiki community :-). Personally I don't think this sandboxed approach works but I understand this is a bigger problem than this and this is the current status quo.

(In reply to Jon from comment #12)

We might need to add media queries to handle mobile-specific styling for
Parsoid-specific content. Could you add a patch for this?

A better approach might be to simply wrap them in a media query that targets
larger screens (e.g. take a mobile first approach) - this would solve the
issue with mobile as the existing styles will kick in - sadly however this
will mean the rules are loaded unnecessarily on mobile.
My main worry is this file getting even more bloaty. Would it not make sense
to use skinStyles on this module so that the styles only apply to
Vector/Monobook?

We are moving this to core now: https://gerrit.wikimedia.org/r/#/c/130770/

Without those styles figures etc are completely unstyled. For this reason we'll add this as a styles.common.* module. Individual skins can further tweak the rendering if needed.

Re bloat: The intention is to keep these styles minimal, and only load them on request in a separate RL module.