r106884 MediaWiki - Code Review archive

After testing this code, I can only say: please revert! What is the reason for this commit completely bypassing the discussion?

Not only is this a completely unilateral change that goes completely agains what has been discussed in r105280, it looks horrible to boot. The yellow and blue levels are totally incompatible; the bright yellow completely overpowers the dull blue. No one asked for the font size change and pre-wrap is not applied. The yellow/blue is a good premise, but these colors are just thrown together from the old and the new.

Can you please post a screen capture?

Regardless, this obviously incorporates several unrequested/undiscussed changes and directly contradicts the consensus reached in the earlier discussion (to simply swap the French Wikipedia's colors, thereby replacing yellow with blue and retaining the longstanding default usage of green).

So yes, please revert (pending further discussion).

Discussion among whom? This is all stylistic changes that any wiki (including the English Wikipedia) is free to override locally when the software is finally pushed live.

If there are substantive issues with this change, please discuss them (on this revision or in a bug or on the wikitech-l mailing list). Discussion about color blindness/accessibility/etc. issues that may arise would be great. But any particular wiki's views on changing the diff color for purely historical or aesthetic reasons aren't really relevant here. Wikis retain their autonomy and sovereignty to easily override this (as consensus dictates).

I was referring to the r105280 discussion (which Edokter cited above), not to one held at a wiki.

Ah, my mistake then! Apologies.

I'd seen the thread at the English Wikipedia village pump (which also heavily discussed the French Wikipedia's design, I think) and thought you were referring to that.

Either way, I'm not sure how much concern needs to be taken here. I didn't mind the old colors and I wouldn't mind better colors. And, as I said, the worst case is that individual wikis (or individual users) decide to simply switch it back for themselves.

I would actually think that making the color scheme fit in with the Vector theme (that's the default now, isn't it?) would be a greater goal than it seems to be.

I, too, didn't mind the old colors and wouldn't mind better colors. The issue is that the new colors introduced via this code differ from those for which consensus was established (and while I wasn't citing the English Wikipedia village pump discussion, it led to the same conclusion).

And even if the yellow/blue color scheme is preferable, this is a flawed implementation (as discussed below).

Most individual wikis/users never customize such settings, so this is non-trivial.

Revisions are morally "neutral" - there is no objective statement "this version is better than that version." The color green, however, is not morally neutral - it is a strong psychological indicator of "good," "correct," "successful," etc. Having either side show as green is thus problematic: if we color the "old" diff green, then we are saying that the old diff is "better". Likewise the inverse: the next revision may not be better.

Using the color "red" or other variants has a similar problem but in the opposite direction: red things are generally perceived as bad or stressful. So we have to avoid red as well.

That leaves us the colors: blue, yellow, purple, and orange. Since colorblindness will cause purples and violets to shift into blues, that doesn't work. Blue and orange are actually ideal and don't shift, but since they are opposites on the color wheel having them stack next to each other causes the eye to see vibrations and cause them to tire. A combination of purple and yellow will have the exact same problem.

That leaves us with a combination of yellow and blue. Since people are already used to yellow on the "previous" diff, and they are already used to a "cool" color for the "next" diff, simply using blue there is the easiest and most conservative choice.

You note above that green can mean "successful". That's a connotation that I believe it carries in this context; it indicates success in editing a page's content. And yes, it also conveys that participating in the editing process is "good" (even if a particular change isn't), which likely encourages new editors to proceed.

This is not to say that the yellow/blue combination isn't worth considering. But no such discussion has occurred (and it's been suggested that the specific shades haven't been carefully determined). Additionally, this code incorporates other undiscussed changes unrelated to the color scheme.

bugzilla:22094 ;-)

Look, I have to know if this will be reverted, otherwise I can't create a (new) patch file. In the mean time, to see my take on the yellow/blue scheme, put these in your CSS and click "Show changes":

/* Diffs */ td.diff-context, td.diff-addedline, td.diff-deletedline {

   white-space: pre-wrap;

} td.diff-context {

   background: #F2F2F2;

} td.diff-addedline {

   background: #E0ECFF;

} td.diff-deletedline {

   background: #FCF8CC;

} td.diff-addedline .diffchange {

   background: #C4D3FF;
   color: #000000;

} td.diff-deletedline .diffchange {

   background: #FFDBA4;
   color: #000000;

}

Thanks! I agree that while the idea has potential, we can do better than this.

The two shades of blue lack sufficient contrast. And Jorm noted that "having [blue and orange] stack next to each other causes the eye to see vibrations and cause them to tire," so that's another issue.

I just realized that the above apparently is your version of the code, yes? (My criticisms stand.)

Can you please post the r106884 version? (I only see it in diff form.)

Forgot to save that one, but given the code above, putting this in your CSS will will show what Brandon's patch would look like:

/* Diffs */
td.diff-context {
    background: #EEE;
}
td.diff-addedline {
    background: #e0ecff;
}
td.diff-deletedline {
    background: #ffffaa;
}
td.diff-addedline .diffchange {
    background: #c4d3ff;
    color: #222222;
}
td.diff-deletedline .diffchange {
    background: #ffd89d;
    color: #000000;
}

Thanks! I agree with your assessment that "the bright yellow completely overpowers the dull blue." This is the exact hexadecimal value used for yellow in the old code, which is stylistically incompatible.

Let's try that again:

/* Diffs */
td.diff-context,
td.diff-addedline,
td.diff-deletedline {
    white-space: pre-wrap;
}
td.diff-context {
    background: #F2F2F2;
}
td.diff-addedline {
    background: #E0ECFF;
}
td.diff-deletedline {
    background: #FCF8CC;
}
td.diff-addedline .diffchange {
    background: #C4D3FF;
    color: #001040;
}
td.diff-deletedline .diffchange {
    background: #FFDBA4;
    color: #302000;
}

Changed the blue highlight:

/* Diffs */
td.diff-context,
td.diff-addedline,
td.diff-deletedline {
    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: #001040;
}
td.diff-deletedline .diffchange {
    background: #FFDBA4;
    color: #302000;
}

This is my final proposal. Please test and discuss.

That's a clear improvement over your previous attempt, but can you please post the r106884 code (in non-diff form)? I'd like to compare it to yours.

The left side's background différentiation is a bit hard to see for me

I adjusted the yellow highlight:

/* Diffs */
td.diff-context,
td.diff-addedline,
td.diff-deletedline {
    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: #001040;
}
td.diff-deletedline .diffchange {
    background: #FFD084;
    color: #302000;
}

Patch at bug 33139 is ready and waiting.

On this screen I can hardly differentiate the color of added text and the color of unchanged text.

To sum up Edokter's various proposals more graphically (number is the comment id) :

(click to zoom 2x)

To me, the left side of 28178 is better than the previous proposals (tho I'm not sure it's tasteful for non colour-blind), but the right side is much less readable.

1. 28168 and #28172 are the same. #28175 only changed the blue highlight background. #28178 (with the hellish yellow) is not my proposal, that is Brandon's. I merely posted that code so others could test it.

My bad

While I like these colors, I don't agree with taking green off limits due to cultural factors. Dark and medium green, yes, but a very faint green doesn't seem to have much cultural "this is good" sense to it. It seems like a case of thinking too hard about stuff :)

I generally like these colors. The goldish/yellowish on the removal side looks fine. If I'd suggest anything I'd maybe take the added line a shade lighter; I'm having trouble distinguishing the two shades of blue.

Can you specify which ones you like?

As they stand committed in trunk.