Page MenuHomePhabricator

[Regression pre-wmf.31] The toolbar for VE is appearing outside the border of the page
Closed, ResolvedPublic

Description

The toolbar for VE is appearing outside the border of the page

Compare the screenshots below:

Beta Cluster:

Production:

Event Timeline

Ryasmeen created this task.May 1 2020, 9:07 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 1 2020, 9:07 PM
Ryasmeen renamed this task from [Regression] The toolbar for VE is appearing outside the border of the page to [Regression pre-wmf.31] The toolbar for VE is appearing outside the border of the page.May 1 2020, 9:08 PM
RhinosF1 added a subscriber: RhinosF1.
Jdforrester-WMF added a subscriber: Jdforrester-WMF.

This is caused by the Vector changes, it seems; the bug goes away if you disable them ("Use Legacy Vector" in skin preferences).

This is caused by the Vector changes, it seems; the bug goes away if you disable them ("Use Legacy Vector" in skin preferences).

Ah that's right. Works as a workaround. Still, it needs to be fixed I guess :)

The rule removed from the modern layout is:
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/skins/Vector/+/f6cbbbfd957b9c9639049455f252190923f07a12/resources/skins.vector.styles/legacy/layout.less#89
Only viewport width >=982px is affected.

Modern will have a new layout, part of which this rule was dropped (atm it seems the padding won't depend on screen width).
The new layout is only active on the beta cluster yet, live servers are not affected.

To resolve this there are a few options:

  1. Temporarily - until the more involved solutions are discussed - the unique padding for >=982px width can be restored. I'll submit a patch for this.
  2. Temporarily VE's negative margin rule can be overridden by the new Vector skin with a more specific selector. This would be an awkward, hacky solution, but feasible.
  3. VE's negative margin trick that reverses the padding of #content can be adjusted. This will require a class like .skin-vector-legacy or skin-vector-1 on the body which is not provided yet by the skin, but will be necessary for gadgets and themes. It's trivial to add, only the class name needs to be decided, then VE's CSS rule adjusted to only affect the legacy layout.
  4. In the long run it would be more reliable to inject the VE header above #content to avoid the padding. That requires wrapping #content in a main element, which necessitates more discussion and a change to VE's code that creates the header.

Change 593954 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] [modern] Temporarily restore the #content (.mw-body) padding for VisualEditor's alignment

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

Change 593955 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/skins/Vector@master] Add CSS class on body to mark the skin version

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

JTannerWMF moved this task from To Triage to Triaged on the VisualEditor board.May 5 2020, 3:25 PM
JTannerWMF moved this task from Backlog to Blocked by others on the Editing-team (Tracking) board.

Hey @ovasileva @Volker_E @Jdlrobson What is our (Web Team's) plan for this?

Hey @ovasileva @Volker_E @Jdlrobson What is our (Web Team's) plan for this?

we have no plans for this right now. If the editing team can let us know how they want to deal with this (for example would Aron's suggested classes help) I can then prioritise this and make sure it's addressed. My preference would be to remove the CSS rules from VisualEditor and/or update them to be more future proofed. This will likely break again when we introduce the fixed width.

I defer to Olga to whether this blocks deployment.

ovasileva added a subscriber: iamjessklein.EditedMay 6 2020, 9:53 AM

I think we're okay for deployment as we won't be deploying to any of the test wikis until fixed width is completed. That said, we will be going to office wiki - @ppelberg, @iamjessklein - is this okay with you? We will be QAing the changes to VE and making fixes as a part of the fixed width work that is a part of the first test wiki deployment T250375: QA VE with new header and collapsible sidebar.

ovasileva triaged this task as Medium priority.May 6 2020, 9:53 AM

skin-vector-2 probably makes sense going forward, although we should use it as little as possible

cc @matmarex

I think we're okay for deployment as we won't be deploying to any of the test wikis until fixed width is completed. That said, we will be going to office wikis - @ppelberg, @iamjessklein - is this okay with you? We will be QAing the changes to VE and making fixes as a part of the fixed width work that is a part of the first test wiki deployment T250375: QA VE with new header and collapsible sidebar.

It isn't ideal, but such is the nature of a shared beta cluster. Is there a way to switch back to Vector-1 using a query string parameter. That way if we (or other teams) want to conduct users tests with a more live-like environment we can do this?

Is there a way to switch back to Vector-1 using a query string parameter.

useskinversion=1
Works as a cookie as well, unlike useskin (T243920).

Change 593954 abandoned by Bartosz Dziewoński:
[modern] Temporarily restore the #content (.mw-body) padding for VisualEditor's alignment

Reason:
Let's do https://gerrit.wikimedia.org/r/593955 instead, I'll submit a patch in VE to use that

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

Change 594793 had a related patch set uploaded (by Bartosz Dziewoński; owner: Bartosz Dziewoński):
[mediawiki/extensions/VisualEditor@master] Update toolbar styles for Vector changes

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

I think we're okay for deployment as we won't be deploying to any of the test wikis until fixed width is completed. That said, we will be going to office wikis - @ppelberg, @iamjessklein - is this okay with you? We will be QAing the changes to VE and making fixes as a part of the fixed width work that is a part of the first test wiki deployment T250375: QA VE with new header and collapsible sidebar.

Thank you for checking, @ovasileva. +1 to what @Esanders commented: so long as the following conditions are met (which it seems they are), we're fine with this being deployed to http://office.wikimedia.org/ [1]:

  • There is a way for us to direct people to URLs where they will not experience the toolbar "bleeding"
  • This issue will be resolved before any additional deployments happen.

  1. Olga, you mentioned "office wikis"; I assumed the plural was a typo. If this is not the case, and if deployed right now, this issue would affect other wikis beyond http://office.wikimedia.org/ please let me know.

  1. Olga, you mentioned "office wikis"; I assumed the plural was a typo. If this is not the case, and if deployed right now, this issue would affect other wikis beyond http://office.wikimedia.org/ please let me know.

Yup, it was a typo. Fixed now!

Change 593955 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Add CSS class on body to mark the skin version

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

I think editing team can handle this for now so moving out of sprint board. Let me know if you need any more assistance from us!

Change 595202 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/VisualEditor@master] Fix editor toolbar positioning for Vector modern layout

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

Change 594793 merged by jenkins-bot:
[mediawiki/extensions/VisualEditor@master] Update toolbar styles for Vector changes

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

Ryasmeen edited projects, added Verified; removed Editing QA.May 11 2020, 5:30 PM
Jdlrobson edited projects, added Vector (Responsive-Vector); removed Vector.
Jdlrobson moved this task from Responsive-Vector to Logo and header on the Vector board.
Jdlrobson edited projects, added Vector; removed Vector (Responsive-Vector).
ppelberg closed this task as Resolved.May 21 2020, 11:39 PM
ppelberg claimed this task.
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptMay 21 2020, 11:39 PM