fix: TextBox text colour could be hard to read

Auto select black/white text based on background colour. Fixed issue with "_exists" call. Added unit tests for PropertyExt. Signed-off-by: Gordon Smith <gordonjsmith@gmail.com>
hpcc-systems · May 21, 2018 · bb15dcc · bb15dcc
1 parent 8d25fa4
commit bb15dcc
Show file tree

Hide file tree

Showing 11 changed files with 155 additions and 22 deletions.
diff --git a/packages/chart/package.json b/packages/chart/package.json
@@ -34,7 +34,7 @@
     "d3-array": "1.2.1",
     "d3-axis": "1.0.8",
     "d3-brush": "1.0.4",
-    "d3-color": "1.0.3",
+    "d3-color": "1.2.0",
     "d3-contour": "1.1.2",
     "d3-format": "1.3.0",
     "d3-geo": "1.6.4",

diff --git a/packages/common/package.json b/packages/common/package.json
@@ -37,6 +37,7 @@
     "d3-array": "1.2.1",
     "d3-brush": "1.0.4",
     "d3-collection": "1.0.4",
+    "d3-color": "1.2.0",
     "d3-dispatch": "1.0.3",
     "d3-drag": "1.2.1",
     "d3-dsv": "1.0.8",

diff --git a/packages/common/src/Palette.ts b/packages/common/src/Palette.ts
@@ -1,5 +1,6 @@
 import * as _colorbrewer from "colorbrewer";
 import { values as d3Values } from "d3-collection";
+import { rgb as d3RGB } from "d3-color";
 import { interpolateLab as d3InterpolateLab } from "d3-interpolate";
 import { interpolateInferno as d3InterpolateInferno, interpolateMagma as d3InterpolateMagma, interpolatePlasma as d3InterpolatePlasma, interpolateViridis as d3InterpolateViridis, scaleLinear as d3ScaleLinear, scaleOrdinal as d3ScaleOrdinal, scaleQuantize as d3ScaleQuantize, scaleSequential as d3ScaleSequential, schemeCategory10 as d3SchemeCategory10, schemeCategory20 as d3SchemeCategory20, schemeCategory20b as d3SchemeCategory20b, schemeCategory20c as d3SchemeCategory20c } from "d3-scale";
 import { select as d3Select } from "d3-selection";
@@ -289,3 +290,7 @@ m_colorbrewer.RdWhGr = {
 
 export const ordinal = fetchOrdinalItem;
 export const rainbow = fetchRainbowItem;
+export function textColor(backgroundColor: string): string {
+    const rgb = d3RGB(backgroundColor);
+    return ((rgb.r * 0.299 + rgb.g * 0.587 + rgb.b * 0.114) > 149) ? "black" : "white";
+}
diff --git a/packages/common/src/PropertyExt.ts b/packages/common/src/PropertyExt.ts
@@ -411,7 +411,9 @@ export class PropertyExt extends Class {
             return this[__prop_data_ + id] !== undefined;
         };
         this[id + "_exists"] = function () {
-            return this[__prop_data_ + id] !== undefined || this[id + "_default"]() !== undefined;
+            if (this[__prop_data_ + id] != null && !(this[__prop_data_ + id] === "" && ext.optional === true)) return true;
+            if (this[id + "_default"]() != null && !(this[id + "_default"]() === "" && ext.optional === true)) return true;
+            return false;
         };
         this[id + "_default"] = function (_) {
             if (!arguments.length) return this[__default_ + id] !== undefined ? this[__default_ + id] : meta.defaultValue;

diff --git a/packages/common/src/Shape.ts b/packages/common/src/Shape.ts
@@ -100,20 +100,28 @@ export class Shape extends SVGWidget {
     dblclick() {
     }
 
-    shape: { (): string; (_: string): Shape; };
-    colorStroke: { (): number; (_: number): Shape; };
-    colorFill: { (): string; (_: string): Shape; };
-    radius: { (): number; (_: number): Shape; };
-    tooltip: { (): string; (_: string): Shape; };
-
 }
 Shape.prototype._class += " common_Shape";
 
+export interface Shape {
+    shape(): string;
+    shape(_: string): this;
+    colorStroke(): number;
+    colorStroke(_: number): this;
+    colorFill(): string;
+    colorFill(_: string): this;
+    colorFill_exists(): boolean;
+    radius(): number;
+    radius(_: number): this;
+    tooltip(): string;
+    tooltip(_: string): this;
+}
+
 Shape.prototype.publish("shape", "circle", "set", "Shape Type", ["circle", "square", "rect", "ellipse"], { tags: ["Private"] });
 Shape.prototype.publish("width", 24, "number", "Width", null, { tags: ["Private"] });
 Shape.prototype.publish("height", 24, "number", "Height", null, { tags: ["Private"] });
-Shape.prototype.publish("colorStroke", null, "html-color", "Stroke Color", null, { tags: ["Private"] });
-Shape.prototype.publish("colorFill", null, "html-color", "Fill Color", null, { tags: ["Private"] });
+Shape.prototype.publish("colorStroke", "", "html-color", "Stroke Color", null, { optional: true });
+Shape.prototype.publish("colorFill", "", "html-color", "Fill Color", null, { optional: true });
 Shape.prototype.publish("radius", null, "number", "Radius", null, { tags: ["Private"] });
 Shape.prototype.publish("tooltip", "", "string", "Tooltip", null, { tags: ["Private"] });
 

diff --git a/packages/common/src/Text.ts b/packages/common/src/Text.ts
@@ -13,7 +13,14 @@ export class Text extends SVGWidget {
     enter(domNode, element) {
         super.enter(domNode, element);
         delete this._prevHash;
-        this._textElement = element.append("text");
+        this._textElement = element.append("text")
+            .on("click", () => {
+                this.click();
+            })
+            .on("dblclick", () => {
+                this.dblclick();
+            })
+            ;
     }
 
     _prevHash;
@@ -68,15 +75,31 @@ export class Text extends SVGWidget {
         }
     }
 
-    text: { (): string; (_: string): Text; };
-    fontFamily: { (): string; (_: string): Text; };
-    fontSize: { (): number; (_: number): Text; };
-    anchor: { (): "start" | "middle" | "end"; (_: "start" | "middle" | "end"): Text; };
-    colorFill: { (): string; (_: string): Text; };
-    rotation: { (): number; (_: number): Text; };
+    click() {
+    }
+
+    dblclick() {
+    }
 }
 Text.prototype._class += " common_Text";
 
+export interface Text {
+    text(): string;
+    text(_: string): this;
+    fontFamily(): string;
+    fontFamily(_: string): this;
+    fontSize(): number;
+    fontSize(_: number): this;
+    anchor(): "start" | "middle" | "end";
+    anchor(_: "start" | "middle" | "end"): this;
+    colorFill(): string;
+    colorFill(_: string): this;
+    colorFill_default(): string;
+    colorFill_default(_: string): this;
+    rotation(): number;
+    rotation(_: number): Text;
+}
+
 Text.prototype.publish("text", "", "string", "Display Text", null, { tags: ["Basic"] });
 Text.prototype.publish("fontFamily", null, "string", "Font Family", null, { tags: ["Intermediate"], optional: true });
 Text.prototype.publish("fontSize", null, "number", "Font Size (px)", null, { tags: ["Intermediate"] });

diff --git a/packages/common/src/TextBox.ts b/packages/common/src/TextBox.ts
@@ -1,3 +1,4 @@
+import { textColor } from "./Palette";
 import { Shape } from "./Shape";
 import { SVGWidget } from "./SVGWidget";
 import { Text } from "./Text";
@@ -22,7 +23,14 @@ export class TextBox extends SVGWidget {
                 this.dblclick();
             })
             ;
-        this._text = new Text();
+        this._text = new Text()
+            .on("click", () => {
+                this.click();
+            })
+            .on("dblclick", () => {
+                this.dblclick();
+            })
+            ;
     }
 
     padding(_) {
@@ -54,6 +62,7 @@ export class TextBox extends SVGWidget {
         if (this._prevHash !== hash) {
             this._prevHash = hash;
             this._text
+                .colorFill_default(this._shape.colorFill_exists() ? textColor(this._shape.colorFill()) : undefined)
                 .render()
                 ;
             const textBBox = this._text.getBBox(true);

diff --git a/packages/form/package.json b/packages/form/package.json
@@ -32,7 +32,7 @@
   "devDependencies": {
     "concurrently": "3.5.1",
     "d3-brush": "1.0.4",
-    "d3-color": "1.0.3",
+    "d3-color": "1.2.0",
     "d3-drag": "1.2.1",
     "d3-scale": "1.0.6",
     "d3-selection": "1.1.0",
@@ -59,4 +59,4 @@
     "url": "https://github.com/hpcc-systems/Visualization/issues"
   },
   "homepage": "https://github.com/hpcc-systems/Visualization"
-}
+}
diff --git a/tests/test-common/package.json b/tests/test-common/package.json
@@ -12,6 +12,7 @@
   "devDependencies": {
     "@types/chai": "3.5.2",
     "@types/mocha": "2.2.41",
+    "concurrently": "3.5.1",
     "css-loader": "0.26.1",
     "file-loader": "1.1.5",
     "mocha-chrome": "1.0.3",
@@ -25,8 +26,10 @@
     "clean": "rimraf lib* && rimraf types && rimraf dist",
     "compile-es6": "tsc --module es6 --outDir ./lib-es6",
     "compile-es6-watch": "npm run compile-es6 -- -w",
-    "bundle-umd": "webpack",
-    "build": "npm run compile-es6 && npm run bundle-umd",
+    "bundle": "webpack",
+    "bundle-watch": "npm run bundle -- -w",
+    "watch": "concurrently --kill-others \"npm run compile-es6-watch\" \"npm run bundle-watch\"",
+    "build": "npm run compile-es6 && npm run bundle",
     "test": "mocha-chrome ./test.html"
   }
 }
diff --git a/tests/test-common/src/index.spec.ts b/tests/test-common/src/index.spec.ts
@@ -1,3 +1,4 @@
 import "es6-promise/auto";  // polyfill Promise on IE
 
+import "./propertyExt.spec";
 import "./common.spec";
diff --git a/tests/test-common/src/propertyExt.spec.ts b/tests/test-common/src/propertyExt.spec.ts
@@ -0,0 +1,81 @@
+import { PropertyExt } from "@hpcc-js/common";
+import { expect } from "chai";
+
+class TestClass extends PropertyExt {
+}
+
+interface TestClass {
+    str(): string;
+    str(_: string): this;
+    str_default(): string;
+    str_default(_: string): this;
+    str_exists(): boolean;
+    str_modified(): boolean;
+    str_reset(): void;
+}
+TestClass.prototype.publish("str", "SomeStr", "string", "Test String");
+TestClass.prototype.publish("num", 42, "number", "Test Number");
+TestClass.prototype.publish("numZero", 0, "number", "Test Number");
+TestClass.prototype.publish("optStr1", "", "string", "Optional String", undefined, { optional: true });
+TestClass.prototype.publish("optStr2", null, "string", "Optional String", undefined, { optional: true });
+TestClass.prototype.publish("optStr3", undefined, "string", "Optional String", undefined, { optional: true });
+
+describe("@hpcc-js/common", () => {
+    describe.only("PropertyExt", () => {
+        function testProp(propID: string, defValue: any, newValue: any) {
+            const testClass: any = new TestClass();
+            expect(testClass[propID + "_modified"](), `${propID}: 1`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 2`).to.be.true;
+            expect(testClass[propID](), `${propID}: 3`).to.equal(defValue);
+            expect(testClass[propID + "_default"](), `${propID}: 4`).to.equal(defValue);
+            testClass[propID + "_reset"]();
+            expect(testClass[propID + "_modified"](), `${propID}: 5`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 6`).to.be.true;
+            expect(testClass[propID](), `${propID}: 7`).to.equal(defValue);
+            expect(testClass[propID + "_default"](), `${propID}: 8`).to.equal(defValue);
+            testClass[propID](newValue);
+            expect(testClass[propID + "_modified"](), `${propID}: 9`).to.be.true;
+            expect(testClass[propID + "_exists"](), `${propID}: 10`).to.be.true;
+            expect(testClass[propID](), `${propID}: 11`).to.equal(newValue);
+            expect(testClass[propID + "_default"](), `${propID}: 12`).to.equal(defValue);
+            testClass[propID + "_reset"]();
+            expect(testClass[propID + "_modified"](), `${propID}: 13`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 14`).to.be.true;
+            expect(testClass[propID](), `${propID}: 15`).to.equal(defValue);
+            expect(testClass[propID + "_default"](), `${propID}: 16`).to.equal(defValue);
+            testClass[propID](defValue);
+            expect(testClass[propID + "_modified"](), `${propID}: 17`).to.be.true;
+            expect(testClass[propID + "_exists"](), `${propID}: 18`).to.be.true;
+            expect(testClass[propID](), `${propID}: 19`).to.equal(defValue);
+            expect(testClass[propID + "_default"](), `${propID}: 20`).to.equal(defValue);
+        }
+        it("Basic String", () => {
+            testProp("str", "SomeStr", "Some Other Str");
+        });
+        it("Basic Number", () => {
+            testProp("num", 42, 48);
+        });
+        it("Zero Default Number", () => {
+            testProp("numZero", 0, 48);
+        });
+        function testOptional(propID: string, exists: boolean) {
+            const testClass: any = new TestClass();
+            expect(testClass[propID + "_modified"](), `${propID}: 1`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 2`).to.equal(exists);
+            testClass[propID + "_reset"]();
+            expect(testClass[propID + "_modified"](), `${propID}: 3`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 4`).to.equal(exists);
+            testClass[propID]("someStr");
+            expect(testClass[propID + "_modified"](), `${propID}: 5`).to.be.true;
+            expect(testClass[propID + "_exists"](), `${propID}: 6`).to.be.true;
+            testClass[propID + "_reset"]();
+            expect(testClass[propID + "_modified"](), `${propID}: 7`).to.be.false;
+            expect(testClass[propID + "_exists"](), `${propID}: 8`).to.equal(exists);
+        }
+        it("Optional", () => {
+            testOptional("optStr1", false);
+            testOptional("optStr2", false);
+            testOptional("optStr3", false);
+        });
+    });
+});