Skip to content

Commit

Permalink
fix: adding error message to color-parsing function (microsoft#1724)
Browse files Browse the repository at this point in the history
* adding error message

* adding and updating tests
  • Loading branch information
nicholasrice authored May 2, 2019
1 parent 9bc6c6a commit 25f04ba
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { colorMatches, contrast, isValidColor } from "./common";
import { colorMatches, contrast, isValidColor, parseColorString } from "./common";
import { ColorRGBA64 } from "@microsoft/fast-colors";

describe("isValidColor", (): void => {
test("should return true when input is a hex color", (): void => {
Expand All @@ -13,8 +14,12 @@ describe("isValidColor", (): void => {
});

describe("colorMatches", (): void => {
test("should return false if arguments are not colors", (): void => {
expect(colorMatches("dksfjd", "weeeeeeee")).toBeFalsy();
test("should throw arguments are not colors", (): void => {
expect(
(): void => {
colorMatches("dksfjd", "weeeeeeee");
}
).toThrow();
});

test("should return true if colors are the same", (): void => {
Expand All @@ -36,14 +41,59 @@ describe("colorMatches", (): void => {
});
});

describe("parseColorHexRGB", (): void => {
test("should parse #RGB color strings", (): void => {
expect(parseColorString("#FFF") instanceof ColorRGBA64).toBe(true);
});
test("should parse #RRGGBB color strings", (): void => {
expect(parseColorString("#001122") instanceof ColorRGBA64).toBe(true);
});
test("should throw if the color is a malformed shothand hex", (): void => {
expect(
(): void => {
parseColorString("#GGG");
}
).toThrow();
});
test("should throw if the color is a malformed hex", (): void => {
expect(
(): void => {
parseColorString("#zzzzzz");
}
).toThrow();
});
test("should throw if the color is a malformed rgb", (): void => {
expect(
(): void => {
parseColorString("rgb(256, 244, 30)");
}
).toThrow();
});
test("should throw if the color is a rgba", (): void => {
expect(
(): void => {
parseColorString("rgba(255, 244, 30, 1)");
}
).toThrow();
});
});

describe("contrast", (): void => {
test("should return the contrast between two colors", (): void => {
expect(contrast("#000", "#FFF")).toBe(21);
expect(contrast("#000000", "#FFFFFF")).toBe(21);
expect(contrast("rgb(0, 0, 0)", "rgb(255, 255, 255)")).toBe(21);
});
test("should return -1 if either color cannot be converted to a color", (): void => {
expect(contrast("oogabooga", "#000")).toBe(-1);
expect(contrast("#000", "oogabooga")).toBe(-1);
test("should throw if either color cannot be converted to a color", (): void => {
expect(
(): void => {
contrast("oogabooga", "#000");
}
).toThrow();
expect(
(): void => {
contrast("#000", "oogabooga");
}
).toThrow();
});
});
33 changes: 16 additions & 17 deletions packages/fast-components-styles-msft/src/utilities/color/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,15 +114,19 @@ export function swatchFamilyToSwatchRecipeFactory<T extends SwatchFamily>(
}

/**
* Converts a color string into a ColorRGBA64 instance. Returns null if the string cannot be converted.
* Converts a color string into a ColorRGBA64 instance.
* Supports #RRGGBB and rgb(r, g, b) formats
*/
export function parseColorString(color: string): ColorRGBA64 | null {
return isColorStringHexRGB(color)
? parseColorHexRGB(color)
: isColorStringWebRGB(color)
? parseColorWebRGB(color)
: null;
export function parseColorString(color: string): ColorRGBA64 {
if (isColorStringHexRGB(color)) {
return parseColorHexRGB(color);
} else if (isColorStringWebRGB(color)) {
return parseColorWebRGB(color);
}

throw new Error(
`${color} cannot be converted to a ColorRGBA64. Color strings must be one of the following formats: "#RGB", "#RRGGBB", or "rgb(r, g, b)"`
);
}

/**
Expand All @@ -138,10 +142,7 @@ export function isValidColor(color: string): boolean {
* Supports #RRGGBB and rgb(r, g, b) formats
*/
export function colorMatches(a: string, b: string): boolean {
const alpha: ColorRGBA64 | null = parseColorString(a);
const beta: ColorRGBA64 | null = parseColorString(b);

return alpha !== null && beta !== null && alpha.equalValue(beta);
return parseColorString(a).equalValue(parseColorString(b));
}

/**
Expand All @@ -150,10 +151,10 @@ export function colorMatches(a: string, b: string): boolean {
*/
export const contrast: (a: string, b: string) => number = memoize(
(a: string, b: string): number => {
const alpha: ColorRGBA64 | null = parseColorString(a);
const beta: ColorRGBA64 | null = parseColorString(b);
const alpha: ColorRGBA64 = parseColorString(a);
const beta: ColorRGBA64 = parseColorString(b);

return alpha === null || beta === null ? -1 : contrastRatio(alpha, beta);
return contrastRatio(alpha, beta);
},
(a: string, b: string): string => a + b
);
Expand All @@ -163,7 +164,5 @@ export const contrast: (a: string, b: string) => number = memoize(
* Supports #RRGGBB and rgb(r, g, b) formats
*/
export function luminance(color: any): number {
const parsedColor: ColorRGBA64 | null = parseColorString(color);

return parsedColor === null ? -1 : rgbToLuminance(parsedColor);
return rgbToLuminance(parseColorString(color));
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import defaultDesignSystem, {
DesignSystemResolver,
} from "../../design-system";
import { clamp } from "lodash-es";
import { colorMatches, contrast, luminance, Swatch } from "./common";
import { colorMatches, contrast, isValidColor, luminance, Swatch } from "./common";
import { neutralForegroundDark, neutralForegroundLight } from "./neutral-foreground";
import { ColorPalette, ColorRGBA64 } from "@microsoft/fast-colors";
import { accentPalette, neutralPalette } from "../design-system";
Expand Down Expand Up @@ -47,6 +47,10 @@ export function findSwatchIndex(
swatch: Swatch
): DesignSystemResolver<number> {
return (designSystem: DesignSystem): number => {
if (!isValidColor(swatch)) {
return -1;
}

const colorPalette: Palette = palette(paletteType)(designSystem);
const index: number = colorPalette.indexOf(swatch);

Expand All @@ -55,7 +59,10 @@ export function findSwatchIndex(
? index
: colorPalette.findIndex(
(paletteSwatch: Swatch): boolean => {
return colorMatches(swatch, paletteSwatch);
return (
isValidColor(paletteSwatch) &&
colorMatches(swatch, paletteSwatch)
);
}
);
};
Expand All @@ -71,12 +78,17 @@ export function findClosestSwatchIndex(
): DesignSystemResolver<number> {
return (designSystem: DesignSystem): number => {
const index: number = findSwatchIndex(paletteType, swatch)(designSystem);
let swatchLuminance: number;

if (index !== -1) {
return index;
}

const swatchLuminance: number = luminance(swatch);
try {
swatchLuminance = luminance(swatch);
} catch (e) {
swatchLuminance = -1;
}

if (swatchLuminance === -1) {
return 0;
Expand Down

0 comments on commit 25f04ba

Please sign in to comment.