Fandom Developers Wiki
Advertisement

This is the talk page for discussing improvements to the Colors page.

Code Review[]

I've looked at this and I have a few of questions/notes:

  • You're adding a "getStyle" function to jQuery. What is the reason for that? $node.css('borderTopColor') already returns the computed style. The function also does not handle the case where the jQuery is empty.
    • Sourcing the border from the top of the page is unreliable. The wiki's local sheet may disable the border entirely, you're going to have to check every page edge, the rail modules, and the DIVs in the footer. Then there's edit pages. It'd be more reliable to calculate it the same way Wikia does from the page color.
  • You've reimplemented jquery.colorUtil instead of importing and using that. Is there a specific reason for that? It wouldn't complicate the interface much, just add a promise to dev.Colors then anyone using the script can do var promise = $.getScript().then(function() { return dev.Colors; }), the promise can be replaced with the real module when it's ready and the promise itself will resolve to the module object.
  • There aren't enough utility functions, you need:
    • saturate
    • rotateHue
    • blend
  • Your "isBright" disagrees with Wikia's, Wikia's uses >= 0.5.
  • Your "brighten" doesn't clamp negative values, if I brighten black by -0.1 then it'll go negative instead of clamping at zero.
  • You're memoizing but you aren't constraining the size of the cache.
  • There's no handling for rgba/hsla
  • This only works in Oasis, Monobook doesn't use SASS.
  • The stylesheet parser is too simple, it only substitutes predetermined values. For ReferencePopups, I need to desaturate and lighten the page color (dark wiki) or darken and blend (light wiki) [background color] and do something similar to come up with the border color, then substitute those values into a mini-sheet, not the raw SASS ones. [I'm also considering making the background into a faint striped gradient which will need a lot of these computations] I'd suggest taking a variable map and substituting arbitrary values:
function replace(sheet, variables) {
    variables = (variables ? $.extend({}, sassParams, variables) : sassParams);
    return sheet.replace(/\$([A-Za-z\-_][A-Za-z0-9\-_]*)/g, function(match, p1) {
        if (variables.hasOwnProperty(p1)) {
            return hexFormat(variables[p1]);
        }
        return match;
    });
}
  • IMHO, it would make more sense to provide an OOP API, define a Color class and stick the functions on that: var hex = new dev.Color.Color('blue').saturate(-0.1).brighten(0.1).rotateHue(180).toString();

Lunarity 19:41, November 3, 2012 (UTC)

As I wrote in the intro: This library is still a bit simple. It's just good enough for what I've been using it. As a general-purpose library it needs a bit of work. :)
  • I defined $.getStyle because it did not seem to me that jQuery can retrieve a computed style. It reproduceably failed in my experiments. Filtering the color out of the page is a dirty hack to begin with of course. I would have removed that as soon as I had looked up the way Wikia uses to calculate the border-color. Thanks for doing that! :)
  • I'm re-implementing large parts of $.colorUtil because I want to
    1. remove a dependency,
    2. provide Wikia-specific functionality and
    3. eventually expose more methods than $.colorUtil does - even so the list is currently rather short. Again: This library is still very young.
  • I used 0.5 to determine brightness in the very first version, but I got odd results on some test pages. It's possible that those odd results were the consequence of local CSS hacks...
  • saturate and blend are good suggestions. I'm not so sure about rotateHue. I can't really think of a use case...
  • the memoizing is only meant for the default colors. That limitation is not in the code yet. I'll add that.
  • adding a variable map to replace and creating a Color class are also good suggestions.
pecoes 20:14, November 03, 2012 (UTC)
  • $node.css('name') does a getComputedStyle internally (that's what it says, at least), but also applies a bunch of filters (the filters are supposed to fix browser bugs). Can you link me to a page where it wasn't working? That seems rather odd.
  • For the luminance, 0.5 is right in the middle so odd results are likely unavoidable since it is, technically, neither light or dark. That gets worse if local CSS nudges it more one way or the other. You could add an isWeird that returns true for lum > 0.45 && lum < 0.55, although I'm not sure what you'd actually do with that information as far as color logic goes.
  • rotateHue is used for calculating contrasting and complementary colors. rotateHue(180) gets the opposite color for maximum contrast. You can also do triad complements by rotating by 180 then rotating again by +/- X (e.g. 30) and so on. I'm not sure it'd be particularly useful, but without hue rotation your API would lack the ability to manipulate the H part of HSL which is a rather obvious gap.
Lunarity 20:57, November 3, 2012 (UTC)
Do you happen to know how Wikia calculates text- and border-color? -- pecoes 18:13, November 04, 2012 (UTC)
Nevermind. I found the colors here http://trac.wikia-code.com/browser/wikia/trunk/skins/oasis/css/core/color.scss
Writing a mix-function that behaves exactly like Sass' mix-function was tricky, but I think I succeeded :) -- pecoes 00:02, November 05, 2012 (UTC)

Problems[]

Two problems I've found:

  • transparent causes a parsing error
  • Short hex codes cause a parsing error ("#000")

Also, the parsing error isn't very helpful. It may be a good idea to include the invalid string in the exception's message so that people can see exactly what caused the problem. Lunarity 03:31, November 7, 2012 (UTC)

I've no idea what to do with "transparent".
pecoes 07:33, November 07, 2012 (UTC)
jquery.colorUtil maps it to white, you could do that but use isBright to map it to black on dark wikis, this is rather arbitrary though. Alternatively, you could just operate on quadruples to include the alpha channel in the color information, then you can just map transparent to [0,0,0,0] rgba(0,0,0,0). Lunarity 12:46, November 7, 2012 (UTC)
Mapping it to white is a not a good idea. In the context of Wikia "transparent" usually means there's a background-image instead of a color.
I'd also rather not add alpha channels to the library because I'd have to rewrite pretty much everything. Wikia does not seem to use rgba or hsla anywhere, so I don't think it's need either. -- pecoes 14:11, November 07, 2012 (UTC)
Wikia doesn't, Oasis was created before RGBA support became common, although IE9 still doesn't support it. I'm using RGBA in RP though, I had to create wrappers to deal with transparent and RGBA. There's also the possibility that sites can use rgba in their custom CSS. The problems I had were with Monobook, I was sniffing the page for the color scheme which is where transparent was giving me problems.
Alpha is a fully independent channel. The value is unchanged by the HSL/RGB conversion, it has no effect on any of the logic except the mix, parse and hex functions. The only problem I see is that hex won't work, it needs to be smart enough to check for alpha===0: transparent, alpha===1: hex, else rgba(r,g,b,a). It doesn't really matter though, I've already worked around it. Lunarity 04:28, November 8, 2012 (UTC)
"although IE9 still doesn't support it"
"it" meaning RGBA? Because it looks to me like IE9 claims to support RGBA colors: http://msdn.microsoft.com/en-us/ie/hh393506.aspx (CSS3 Color Module) 20px_Rin_Tohsaka_Avatar.png Mathmagician ƒ(♫) 04:01 UTC, Friday, 9 November 2012

(Reset indent) Yeah, actually it does work in IE9, but only in "standards mode". Doesn't work in IE8 though. Lunarity 05:16, November 9, 2012 (UTC)

Clarification: Isn't all of Wikia in "standards mode"? <!DOCTYPE html> seems to be included in the HTML at the top of every Wikia page. That is, IE9 users on Wikia will see RGBA functioning properly? That is, out of all of Wikia's supported browsers, the only one which does not support RGBA is IE8 (which to me is a negligible browser)? 20px_Rin_Tohsaka_Avatar.png Mathmagician ƒ(♫) 05:32 UTC, Friday, 9 November 2012
Correct, Wikia use HTML5 so they're compliant with it. Unfortunately, IE8 isn't likely to go away any time soon though (it's the new IE6), MS decided that IE9 would be Vista+ only so everyone who is still on XP has a good chance of using IE8. Lunarity 05:37, November 9, 2012 (UTC)
Google is dropping support for IE8 for online apps and services on November 15th, per their policy of only supporting the current version and the one directly before it. Hopefully IE8 will go away sooner rather than later. 20px_Rin_Tohsaka_Avatar.png Mathmagician ƒ(♫) 06:26 UTC, Friday, 9 November 2012

Broken[]

The current code is non-functional. parse returns a tuple instead of a color object and it can't parse color names at all. It crashes itself because it needs both of those things internally as well. Lunarity 10:59, November 7, 2012 (UTC)

Oh, I'm sorry! I copypasted my test-code from the console into the page. The test-code didn't use the Color object, of course. -- pecoes 11:12, November 07, 2012 (UTC)

The regex you wrote for parsing 3 hex was not specific enough; it was catching 6 hex and interpreting it as 3 so I was getting green instead of grey and other weirdness. I had to revert it to the previously functional version to fix RP. The regex needs to be improved to not catch 6 in the 3 match, like adding $, \b or using more than one regex. Lunarity 12:56, November 7, 2012 (UTC)

I've fixed it. The regex wasn't grouped correctly, the $ was only applying to the rgb part, not all 3. Lunarity 14:05, November 7, 2012 (UTC)
Thanks. That'll teach me to do a quick hack! :) -- pecoes 14:12, November 07, 2012 (UTC)

rotate/saturate/lighten[]

Hi Lunarity! Please don't take this the wrong way, but I just removed your explanations of these three functions. It seems to me they do more to complicate the matter than to clarify it. While they were certainly technically correct, they were harder to understand than the purpose and usage of the functions themselves. When the explanations are more complicated than what they try to explain, then the explanations are a problem. They also suggest that hue/saturation/lightness are somehow superior to these three functions, while in real life the contrary is the case. When designing for Wikia you never build things from scratch but always on top of existing designs. That makes the relative functions clearly superior. I do understand that strictly speaking everything can be done with the absolute functions, while not everything can be done with the relative functions, but the relative functions will be (and should be) everybody's first choice regardless. -- pecoes 07:51, November 10, 2012 (UTC)

Incorrect $gradient color[]

The color of dev.colors.wikia.gradient is slightly off. For example, here on Dev Wiki, buttons are #6b7c92 on hover, but the variable is #6b7b91.—YXTQWFclimbTheStairs 00:48, April 5, 2020 (UTC)

Advertisement