From cc29bd9e884df8433923f943a508b14c9485e55a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Rivi=C3=A8re?= Date: Thu, 24 Aug 2023 11:55:28 +0200 Subject: [PATCH 1/2] when we specify fill: "currentColor", this.fill is undefined, but we still want to pass "currentColor" as a hint to the symbol scale closes #1462 --- src/marks/dot.js | 6 ++++- test/output/symbolSetFill.svg | 43 +++++++++++++++++++++++++++++++++ test/output/symbolSetStroke.svg | 43 +++++++++++++++++++++++++++++++++ test/plots/index.ts | 1 + test/plots/symbol-sets.ts | 15 ++++++++++++ 5 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 test/output/symbolSetFill.svg create mode 100644 test/output/symbolSetStroke.svg create mode 100644 test/plots/symbol-sets.ts diff --git a/src/marks/dot.js b/src/marks/dot.js index 2d969444c6..faa5d71241 100644 --- a/src/marks/dot.js +++ b/src/marks/dot.js @@ -57,7 +57,11 @@ export class Dot extends Mark { if (symbolChannel) { const {fill: fillChannel, stroke: strokeChannel} = channels; symbolChannel.hint = { - fill: fillChannel ? (fillChannel.value === symbolChannel.value ? "color" : "currentColor") : this.fill, + fill: fillChannel + ? fillChannel.value === symbolChannel.value + ? "color" + : "currentColor" + : this.fill ?? options.fill, stroke: strokeChannel ? (strokeChannel.value === symbolChannel.value ? "color" : "currentColor") : this.stroke }; } diff --git a/test/output/symbolSetFill.svg b/test/output/symbolSetFill.svg new file mode 100644 index 0000000000..a9301955ab --- /dev/null +++ b/test/output/symbolSetFill.svg @@ -0,0 +1,43 @@ + + + + + + + + + + + + + circle + cross + diamond + square + star + triangle + wye + + + + + + + + + + + \ No newline at end of file diff --git a/test/output/symbolSetStroke.svg b/test/output/symbolSetStroke.svg new file mode 100644 index 0000000000..1e96881b18 --- /dev/null +++ b/test/output/symbolSetStroke.svg @@ -0,0 +1,43 @@ + + + + + + + + + + + + + circle + cross + diamond + square + star + triangle + wye + + + + + + + + + + + \ No newline at end of file diff --git a/test/plots/index.ts b/test/plots/index.ts index 7660b3f473..1e022ab259 100644 --- a/test/plots/index.ts +++ b/test/plots/index.ts @@ -287,6 +287,7 @@ export * from "./stargazers-hourly-group.js"; export * from "./stargazers-hourly.js"; export * from "./stargazers.js"; export * from "./stocks-index.js"; +export * from "./symbol-sets.js"; export * from "./text-overflow.js"; export * from "./this-is-just-to-say.js"; export * from "./time-axis.js"; diff --git a/test/plots/symbol-sets.ts b/test/plots/symbol-sets.ts new file mode 100644 index 0000000000..c9cd5cd070 --- /dev/null +++ b/test/plots/symbol-sets.ts @@ -0,0 +1,15 @@ +import * as Plot from "@observablehq/plot"; + +export async function symbolSetFill() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + fill: "currentColor", + symbol: (d, i) => i + }).plot(); +} + +export async function symbolSetStroke() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + stroke: "currentColor", + symbol: (d, i) => i + }).plot(); +} From 6d898df31914b50b6670e51330252dcd0551298a Mon Sep 17 00:00:00 2001 From: Mike Bostock Date: Thu, 24 Aug 2023 09:35:56 -0700 Subject: [PATCH 2/2] materialize default stroke, too --- src/marks/dot.js | 8 ++- test/marks/dot-test.js | 68 +++++++++++++++++++++ test/output/symbolSetFillColor.html | 85 +++++++++++++++++++++++++++ test/output/symbolSetStrokeColor.html | 85 +++++++++++++++++++++++++++ test/plots/index.ts | 2 +- test/plots/symbol-set.ts | 29 +++++++++ test/plots/symbol-sets.ts | 15 ----- 7 files changed, 274 insertions(+), 18 deletions(-) create mode 100644 test/output/symbolSetFillColor.html create mode 100644 test/output/symbolSetStrokeColor.html create mode 100644 test/plots/symbol-set.ts delete mode 100644 test/plots/symbol-sets.ts diff --git a/src/marks/dot.js b/src/marks/dot.js index faa5d71241..58e529def2 100644 --- a/src/marks/dot.js +++ b/src/marks/dot.js @@ -61,8 +61,12 @@ export class Dot extends Mark { ? fillChannel.value === symbolChannel.value ? "color" : "currentColor" - : this.fill ?? options.fill, - stroke: strokeChannel ? (strokeChannel.value === symbolChannel.value ? "color" : "currentColor") : this.stroke + : this.fill ?? "currentColor", + stroke: strokeChannel + ? strokeChannel.value === symbolChannel.value + ? "color" + : "currentColor" + : this.stroke ?? "none" }; } } diff --git a/test/marks/dot-test.js b/test/marks/dot-test.js index 635e365a7c..e8b80b1c64 100644 --- a/test/marks/dot-test.js +++ b/test/marks/dot-test.js @@ -111,3 +111,71 @@ it("dot(data, {stroke}) defaults strokeWidth to 1.5 if stroke is defined", () => assert.strictEqual(Plot.dot(undefined, {stroke: "x"}).strokeWidth, 1.5); assert.strictEqual(Plot.dot(undefined, {stroke: null}).strokeWidth, undefined); }); + +it("dot(data, {fill, symbol}) initializes the symbol hint for a constant fill", () => { + assert.deepStrictEqual( + Plot.dot([], { + fill: "currentColor", + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "currentColor", stroke: "none"} + ); + assert.deepStrictEqual( + Plot.dot([], { + fill: "red", + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "red", stroke: "none"} + ); +}); + +it("dot(data, {fill, symbol}) initializes the symbol hint for a channel fill", () => { + assert.deepStrictEqual( + Plot.dot([], { + fill: Plot.indexOf, + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "color", stroke: "none"} + ); + assert.deepStrictEqual( + Plot.dot([], { + fill: Plot.identity, + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "currentColor", stroke: "none"} + ); +}); + +it("dot(data, {stroke, symbol}) initializes the symbol hint for a constant stroke", () => { + assert.deepStrictEqual( + Plot.dot([], { + stroke: "currentColor", + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "none", stroke: "currentColor"} + ); + assert.deepStrictEqual( + Plot.dot([], { + stroke: "red", + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "none", stroke: "red"} + ); +}); + +it("dot(data, {stroke, symbol}) initializes the symbol hint for a channel stroke", () => { + assert.deepStrictEqual( + Plot.dot([], { + stroke: Plot.indexOf, + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "none", stroke: "color"} + ); + assert.deepStrictEqual( + Plot.dot([], { + stroke: Plot.identity, + symbol: Plot.indexOf + }).initialize().channels.symbol.hint, + {fill: "none", stroke: "currentColor"} + ); +}); diff --git a/test/output/symbolSetFillColor.html b/test/output/symbolSetFillColor.html new file mode 100644 index 0000000000..972bee3f07 --- /dev/null +++ b/test/output/symbolSetFillColor.html @@ -0,0 +1,85 @@ +
+
+ + + 0 + + 1 + + 2 + + 3 + + 4 + + 5 + + 6 +
+ + + + + + + + + + + + circle + cross + diamond + square + star + triangle + wye + + + + + + + + + + + +
\ No newline at end of file diff --git a/test/output/symbolSetStrokeColor.html b/test/output/symbolSetStrokeColor.html new file mode 100644 index 0000000000..27c0f78d40 --- /dev/null +++ b/test/output/symbolSetStrokeColor.html @@ -0,0 +1,85 @@ +
+
+ + + 0 + + 1 + + 2 + + 3 + + 4 + + 5 + + 6 +
+ + + + + + + + + + + + circle + cross + diamond + square + star + triangle + wye + + + + + + + + + + + +
\ No newline at end of file diff --git a/test/plots/index.ts b/test/plots/index.ts index 1e022ab259..747dad015d 100644 --- a/test/plots/index.ts +++ b/test/plots/index.ts @@ -287,7 +287,7 @@ export * from "./stargazers-hourly-group.js"; export * from "./stargazers-hourly.js"; export * from "./stargazers.js"; export * from "./stocks-index.js"; -export * from "./symbol-sets.js"; +export * from "./symbol-set.js"; export * from "./text-overflow.js"; export * from "./this-is-just-to-say.js"; export * from "./time-axis.js"; diff --git a/test/plots/symbol-set.ts b/test/plots/symbol-set.ts new file mode 100644 index 0000000000..0e79c3a113 --- /dev/null +++ b/test/plots/symbol-set.ts @@ -0,0 +1,29 @@ +import * as Plot from "@observablehq/plot"; + +export async function symbolSetFill() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + fill: "currentColor", + symbol: Plot.indexOf + }).plot(); +} + +export async function symbolSetStroke() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + stroke: "currentColor", + symbol: Plot.indexOf + }).plot(); +} + +export async function symbolSetFillColor() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + fill: Plot.indexOf, + symbol: Plot.indexOf + }).plot({symbol: {legend: true}}); +} + +export async function symbolSetStrokeColor() { + return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { + stroke: Plot.indexOf, + symbol: Plot.indexOf + }).plot({symbol: {legend: true}}); +} diff --git a/test/plots/symbol-sets.ts b/test/plots/symbol-sets.ts deleted file mode 100644 index c9cd5cd070..0000000000 --- a/test/plots/symbol-sets.ts +++ /dev/null @@ -1,15 +0,0 @@ -import * as Plot from "@observablehq/plot"; - -export async function symbolSetFill() { - return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { - fill: "currentColor", - symbol: (d, i) => i - }).plot(); -} - -export async function symbolSetStroke() { - return Plot.dotX(["circle", "cross", "diamond", "square", "star", "triangle", "wye"], { - stroke: "currentColor", - symbol: (d, i) => i - }).plot(); -}