Page MenuHomePhabricator

Highlight does not cover the menu option entirely in the toolbar on Mobile VE
Closed, ResolvedPublic

Description

Highlight does not cover the menu option entirely in the toolbar on Mobile VE

Production:

Beta:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 23 2018, 11:25 PM

Looks like we need to tweak the height of the toolbar to match the new font size.

Thanks @Ryasmeen for looking into all the VE Mobile issues, I've been fiddling around yesterday and provided a first fix to MobileFrontend. More to come…

@Volker_E I'm not sure what to do with this task. Should it be merged into something else, if you're writing a patch that fixes those issues too?

Ryasmeen added a comment.EditedApr 5 2018, 11:12 PM

okay looks like this is still not completely fixed. I see, it's fixed for the first menu option Text formatting but not for the other ones? Has there actually been any fix for this ?

Screenshots:
Text formatting highlight


Link

@Ryasmeen thanks for the updated screens.
@Deskana Sorry for the delay, no let's keep it in this task. That's a OOUI in mobile VE specific issue. Ed was fixing some of those and I then cared for other affected products. Will provide a patch.

Restricted Application added a project: UI-Standardization. · View Herald TranscriptApr 5 2018, 11:28 PM

Change 435827 had a related patch set uploaded (by VolkerE; owner: VolkerE):
[mediawiki/extensions/VisualEditor@master] Make VE mobile tools use full height

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

That's a VE mobile specific issue.
My patches result in

Change 450580 had a related patch set uploaded (by Esanders; owner: Esanders):
[mediawiki/skins/MinervaNeue@master] Slightly reduce toolbar height when using VE

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

Change 452598 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Tweak toolbar items' height

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

Change 435827 abandoned by Bartosz Dziewoński:
Make VE mobile tools use full height

Reason:
I guess we're going with Ed's approach.

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

We're solving this by overriding the height of mobile editor's toolbar from 3.35em to 3em only when using VisualEditor. The existing VisualEditor styles are written to use a 3em toolbar (same on desktop and mobile) and overriding everything to use 3.35em would require a lot of fragile code (and where does 3.35em come from, anyway?).

On Minerva, which uses a 16px base font size, 3.35em = 53.6px, 3em = 48px, which is still larger than the 44px minimum size I've seen recommended for mobile buttons. (On Vector, which uses a 14px base font size, 3em = 42px.)

Change 452598 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] ve.init.mw.MobileArticleTarget: Tweak toolbar items' height

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

Change 450580 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Slightly reduce toolbar height when using VE

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

Deskana closed this task as Resolved.Aug 16 2018, 1:41 PM
Deskana assigned this task to matmarex.
Deskana triaged this task as Normal priority.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptAug 16 2018, 1:41 PM
Volker_E raised the priority of this task from Normal to Needs Triage.Nov 27 2018, 4:34 AM
Volker_E removed a project: Patch-For-Review.
Volker_E moved this task from Backlog to Done on the UI-Standardization-Kanban board.