Page MenuHomePhabricator

[ICONS] last edited element alignment
Open, MediumPublic

Description

Note: Possibly should be merged with T238681 which is impacted by the same problem

The clock icon + text in the last edited module are not centered vertically, they need to be moved up.

Acceptance criteria

  • Ensure the solution does not clip the text (see T234492)
  • The icon is aligned.

    Developer notes

.mw-ui-icon-small elements have height 1em so this applies to the whole last modified bar. Ideally the clock icon would be a child of the bar, not the bar itself..

Given the regressions we've had with the last modified bar, I suggest we setup storybook to help testing this one.

Details

Related Gerrit Patches:
mediawiki/skins/MinervaNeue : masterLast modified should have line-height 1
mediawiki/skins/MinervaNeue : masterRefactor "last-modified" bar to use flexbox layout.

Event Timeline

Restricted Application removed a project: Patch-For-Review. · View Herald TranscriptSep 17 2019, 10:49 PM
ovasileva triaged this task as Medium priority.Sep 18 2019, 9:41 AM
matmarex removed a subscriber: matmarex.Sep 18 2019, 2:03 PM
Jdlrobson updated the task description. (Show Details)Sep 18 2019, 3:36 PM
Jdlrobson moved this task from Needs triage to MinervaNeue on the Mobile board.Sep 26 2019, 12:11 AM

Change 539310 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Refactor "last-modified" bar to use flexbox layout.

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

This broke cached HTML:


I've minimised the damage in my follow up patch so it looks like this:

Change 539310 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Refactor "last-modified" bar to use flexbox layout.

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

alexhollender reassigned this task from alexhollender to Jdlrobson.EditedSep 30 2019, 2:26 PM

Checked on Beta. The text is still sitting about a pixel too low. The clock and chevron icons are aligned properly.

Change 539928 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Drop content class from last modified bar

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

Change 539979 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Last modified should have line-height 1

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

Change 539979 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Last modified should have line-height 1

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

@Jdlrobson the alignment is correct :) but the bottoms of some letters are getting clipped, which seems to be related to overflow: hidden on the last-modified-bar__text class.

If you want to resolve this task (since it's about alignment) and make a new one for the clipping that's cool with me. Not sure what makes the most sense.

@Jdlrobson the alignment is correct :) but the bottoms of some letters are getting clipped, which seems to be related to overflow: hidden on the last-modified-bar__text class.


If you want to resolve this task (since it's about alignment) and make a new one for the clipping that's cool with me. Not sure what makes the most sense.

That bug is captured in https://phabricator.wikimedia.org/T234492 and I have spent the week arguing about the best way to solve it :)

So that (line-height: 1) might, no, will have even worse outcome in languages like Burmese. Should have thought about this, as we've run into issues in OOUI and circumvented them by reducing padding, instead it should provide
line-height: 18px equiv minimum.

Continuing in T234492

ovasileva closed this task as Resolved.Oct 7 2019, 10:13 AM

Resolving this one then

Jdlrobson reopened this task as Open.Oct 7 2019, 11:32 PM

The patch that fixed this was reverted as it caused T234492