Last modified: 2012-05-03 02:42:46 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; }
Created attachment 9758 [details] Patch for mediawiki.action.history.diff.css
Adding self to CC to track this bug. Not going to poke it before january though.
Applied in r107127. Erwin, thanks for the contribution, and taking Brandon's considerations into account.
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?
I apparently forgot to reopen...
Does that vertical-align: top; serves any specific purpose? Looks like the text already take all the available cell height.
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.
Makes perfect sense. I have applied the change with r109932. Thanks!
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
*** Bug 33139 has been marked as a duplicate of this bug. ***
*** Bug 11374 has been marked as a duplicate of this bug. ***
*** This bug has been marked as a duplicate of bug 11374 ***