Last modified: 2012-05-03 02:42:46 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T35335, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 33335 - New color scheme and text display for diffs
New color scheme and text display for diffs
Status: RESOLVED DUPLICATE of bug 11374
Product: MediaWiki
Classification: Unclassified
History/Diffs (Other open bugs)
unspecified
All All
: Normal enhancement (vote)
: 1.19.0 release
Assigned To: Nobody - You can work on this!
: design
Depends on:
Blocks: 31217
  Show dependency treegraph
 
Reported: 2011-12-22 19:49 UTC by Erwin Dokter
Modified: 2012-05-03 02:42 UTC (History)
6 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Patch for mediawiki.action.history.diff.css (1.12 KB, patch)
2011-12-22 19:49 UTC, Erwin Dokter
Details
Patch for mediawiki.action.history.diff.css - fix missing line (381 bytes, patch)
2012-01-14 12:17 UTC, Erwin Dokter
Details

Description Erwin Dokter 2011-12-22 19:49:27 UTC
Several commits have been done to change the appearence of diffs to accomodate people with color blindness. However, some changes were not discussed. That is why I submit a new proposal that encompasses several changes detailed below.

Color changes:
* The lines in td.diff-deletedline is changed to have a soft yellow background, with the removed text having an amber background and black text in bold.
* The lines in td.diff-addedline is changed to have a light blue background, with the added text having a lavender background and black text in bold.
* The lines in td.diff-context will have a slightly lighter grey background.

Much consideration has been applied to make sure all used colors match in hue and brightness levels. 

Text display changes:
* Instead of only the changed text being displayed with white-space:pre-wrap, the entire line in a cell is. This will show indents and any removed/added spaces, which greatly helps in code review.
* The font-size will be changed from 'smaller' to 88%, which will ensure a proper, legible font-size accross browsers.

Screenshot available at [[Commons:File:ProposedDiffChanges.png]]

To test the new color scheme, put the following in your CSS:
/* Diffs */
td.diff-context,
td.diff-addedline,
td.diff-deletedline {
    font-size: 88%;
    white-space: pre-wrap;
}
td.diff-context {
    background: #F2F2F2;
}
td.diff-addedline {
    background: #E0ECFF;
}
td.diff-deletedline {
    background: #FCF8CC;
}
td.diff-addedline .diffchange {
    background: #B0C8FF;
    color: #000;
}
td.diff-deletedline .diffchange {
    background: #FFD084;
    color: #000;
}
Comment 1 Erwin Dokter 2011-12-22 19:49:53 UTC
Created attachment 9758 [details]
Patch for mediawiki.action.history.diff.css
Comment 2 Antoine "hashar" Musso (WMF) 2011-12-22 22:30:30 UTC
Adding self to CC to track this bug. Not going to poke it before january though.
Comment 3 Rob Lanphier 2011-12-23 00:25:39 UTC
Applied in r107127.  Erwin, thanks for the contribution, and taking Brandon's considerations into account.
Comment 4 Erwin Dokter 2012-01-14 12:17:21 UTC
Created attachment 9855 [details]
Patch for mediawiki.action.history.diff.css - fix missing line

In my previous patch, I have accidentally left out one line. Can this still be added?
Comment 5 Erwin Dokter 2012-01-22 13:00:49 UTC
I apparently forgot to reopen...
Comment 6 Antoine "hashar" Musso (WMF) 2012-01-23 23:40:09 UTC
Does that vertical-align: top; serves any specific purpose? Looks like the text already take all the available cell height.
Comment 7 Erwin Dokter 2012-01-24 00:25:24 UTC
There are situations where one side has a lot of text, while the other side only has one line. That line would dangle in the middle of nowhere instead of on the top of the cell.

Sample diff on enwiki: http://en.wikipedia.org/w/index.php?title=Astrid_Peth&diff=472506437&oldid=472503981

Note that the vertical-align:top has already been implemented locally on enwiki, so you see the intended effect.
Comment 8 Antoine "hashar" Musso (WMF) 2012-01-24 16:26:18 UTC
Makes perfect sense. I have applied the change with r109932. Thanks!
Comment 9 Krinkle 2012-03-01 01:09:05 UTC
Diff style reverted back to status que as of 1.18.0 in r112750.

Bug is still valid and needs a solution, see r112750 commit-msg for details
Comment 10 Krinkle 2012-03-01 01:10:12 UTC
*** Bug 33139 has been marked as a duplicate of this bug. ***
Comment 11 Krinkle 2012-03-01 01:10:19 UTC
*** Bug 11374 has been marked as a duplicate of this bug. ***
Comment 12 Krinkle 2012-03-01 01:11:51 UTC

*** This bug has been marked as a duplicate of bug 11374 ***

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links