Page MenuHomePhabricator

CX: Adjust background color in the translation editor view
Closed, ResolvedPublic

Description

A regression in the translation editor shows the document background in grey instead of white. Note in the example below how the background for the whole image is grey including the top header, the document area (with the original and translated articles) and the tools column.

The expected aspect is for the top header, and the document area (with the original and translated articles) to be white. While the tools column uses a grey background.

Although it is a visual change that does not affect functionality, it makes the tool look broken and makes the layout hard to understand (e.g., differentiating the tools column as a separate element).

Tested on:

  • Browser: Google Chrome Stable.
  • OS: Debian Linux.
  • Wikipedia: Gujarati (guwiki), but happens in all Wikis starting with wmf.12 branch.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 13 2018, 7:19 AM
Pginer-WMF triaged this task as High priority.Jul 13 2018, 8:25 AM
Pginer-WMF renamed this task from CX: Change of background color in Translation to CX: Adjust background color in the translation editor view.Jul 13 2018, 8:31 AM
Pginer-WMF updated the task description. (Show Details)
Pginer-WMF moved this task from Needs Triage to Bugs on the ContentTranslation board.

Change 445585 had a related patch set uploaded (by Petar.petkovic; owner: Petar.petkovic):
[mediawiki/extensions/ContentTranslation@master] Fix colors on CX1

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

The reason behind this color change is that this patch was intended to remove the LESS mixins from our CSS output and thus reduce the payload, but .cx-widget was actual class definition and not used as mixin.

Since ext.cx.common.less is now predominantly a file where mixins and variables are defined, we could split it into ext.cx.mixins.less and ext.cx.variables.less files, reducing the chance of such bugs happening in the future. .cx-widget, being only common CSS class in ext.cx.common.less, can stay there or be moved somewhere else.

The reason behind this color change is that this patch was intended to remove the LESS mixins from our CSS output and thus reduce the payload, but .cx-widget was actual class definition and not used as mixin.

Since ext.cx.common.less is now predominantly a file where mixins and variables are defined, we could split it into ext.cx.mixins.less and ext.cx.variables.less files, reducing the chance of such bugs happening in the future. .cx-widget, being only common CSS class in ext.cx.common.less, can stay there or be moved somewhere else.

Thanks for the fix, and the details provided, @Petar.petkovic. If it helps to plan, discuss and organize the refactoring effort, you can consider turning the last part of your comment into a separate ticket. If doing the refactoring takes less time than creating the ticket, then probably is not worth it. So up to you.

Change 445585 merged by Petar.petkovic:
[mediawiki/extensions/ContentTranslation@master] Fix colors on CX1

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

Thanks for the fix, and the details provided, @Petar.petkovic. If it helps to plan, discuss and organize the refactoring effort, you can consider turning the last part of your comment into a separate ticket. If doing the refactoring takes less time than creating the ticket, then probably is not worth it. So up to you.

I will use this ticket for refactoring. Thus moved to "In progress".

Change 446212 had a related patch set uploaded (by KartikMistry; owner: Petar.petkovic):
[mediawiki/extensions/ContentTranslation@wmf/1.32.0-wmf.12] Fix colors on CX1

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

Change 446212 merged by jenkins-bot:
[mediawiki/extensions/ContentTranslation@wmf/1.32.0-wmf.12] Fix colors on CX1

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

Mentioned in SAL (#wikimedia-operations) [2018-07-17T11:46:00Z] <zfilipin@deploy1001> Synchronized php-1.32.0-wmf.12/extensions/ContentTranslation: SWAT: [[gerrit:446212|Fix colors on CX1 (T199503)]] (duration: 00m 55s)

Change 446596 had a related patch set uploaded (by Petar.petkovic; owner: Petar.petkovic):
[mediawiki/extensions/ContentTranslation@master] Split common LESS file into mixins and variables

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

Pginer-WMF closed this task as Resolved.Jul 24 2018, 9:32 AM