Page MenuHomePhabricator

Line height in drawers is tight (viewing references on mobile)
Closed, ResolvedPublic


For example, references:

image.png (640×358 px, 64 KB)

The line height is overridden to 100%, instead of using the skin default of 140%.

Even on a small screen, there appears to be enough room for this more comfortable line height:

image.png (637×357 px, 60 KB)

Event Timeline

Change 580080 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/extensions/MobileFrontend@master] Remove line-height overrides for drawers

Jdlrobson added a subscriber: alexhollender.

Cc @alexhollender. Pulling into sprint as we'll want to verify all the other drawers in storybook.

Looking at the storybook, only the reference drawer uses the affected selectors (.drawer.text and .drawer p) so this should have no effect on other drawers.

Actually the AMC drawer uses the .drawer p selector, but it already inherits 1.4 line height from the body.

Note that in the storybook not all the Minerva classes are present (for example the 1.4 line height on the body).

JTannerWMF added a subscriber: JTannerWMF.

This task is waiting on code review from the web team.

Change 580080 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove line-height overrides for drawers

@JTannerWMF - review is done. Should we send this back to the Editing team for QA?

Just did a quick QA on the beta cluster. The change looks good:

beta (link)production (link)
en.m.wikipedia.beta.wmflabs.org_wiki_Uganda_Martyrs(iPhone 6_7_8).png (1×750 px, 225 KB)
en.m.wikipedia.org_wiki_%C3%89tude_Op._25,_No._1_(Chopin)(iPhone 6_7_8).png (1×750 px, 180 KB)

Going by the screenshots in T244510 this appears to be a recent regression, which suggests that whatever is generating the HTML of the references, started stripping the paragraph tags? CC @matmarex

I looked at wmf/1.35.0-wmf.15 to see how it was rendering there. There were never paragraph tags, but there was a wrapper with the reference-text class, and that has line-height: 1.4; defined here: ReferencesDrawer.less#L38

Comparison of the DOM structure before and after:

image.png (891×2 px, 149 KB)
image.png (891×2 px, 143 KB)

I think change rEMFR0da51be80f02: Fix ReferencesHtmlScraperGateway showing child references is the reason.

Now that you fixed it differently, I think we should just remove the styling for .reference-text in ReferencesDrawer.less.

Oh dear. I believe our mistake in the patch (done as part of T242899) was that we only tested 1-line references. Thanks a lot for fixing this mistake!

Yes, the .reference-text { line-height: 1.4; } can now be removed.

Moving to editing QA and untagging Readers Web - feel free to add again if there's anything else we can do to help here

Change 583434 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/MobileFrontend@master] ReferencesDrawer: Remove unused styles for .reference-text

Change 583434 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] ReferencesDrawer: Remove unused styles for .reference-text