Skip to content

Commit

Permalink
fix for null scales (#759)
Browse files Browse the repository at this point in the history
  • Loading branch information
mbostock authored Feb 17, 2022
1 parent 4663ef3 commit 36a873c
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 6 deletions.
10 changes: 5 additions & 5 deletions src/legends.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {rgb} from "d3";
import {isObject} from "./options.js";
import {isScaleOptions} from "./options.js";
import {normalizeScale} from "./scales.js";
import {legendRamp} from "./legends/ramp.js";
import {legendSwatches, legendSymbols} from "./legends/swatches.js";
Expand All @@ -13,17 +13,17 @@ const legendRegistry = new Map([
export function legend(options = {}) {
for (const [key, value] of legendRegistry) {
const scale = options[key];
if (isObject(scale)) { // e.g., ignore {color: "red"}
if (isScaleOptions(scale)) { // e.g., ignore {color: "red"}
let hint;
// For symbol legends, pass a hint to the symbol scale.
if (key === "symbol") {
const {fill, stroke = fill === undefined && isObject(options.color) ? "color" : undefined} = options;
const {fill, stroke = fill === undefined && isScaleOptions(options.color) ? "color" : undefined} = options;
hint = {fill, stroke};
}
return value(
normalizeScale(key, scale, hint),
legendOptions(scale, options),
key => isObject(options[key]) ? normalizeScale(key, options[key]) : null
key => isScaleOptions(options[key]) ? normalizeScale(key, options[key]) : null
);
}
}
Expand Down Expand Up @@ -75,7 +75,7 @@ export function Legends(scales, options) {
const legends = [];
for (const [key, value] of legendRegistry) {
const o = options[key];
if (o && o.legend) {
if (o?.legend && (key in scales)) {
const legend = value(scales[key], legendOptions(scales[key], o), key => scales[key]);
if (legend != null) legends.push(legend);
}
Expand Down
11 changes: 10 additions & 1 deletion src/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,16 @@ export function arrayify(data, type) {

// Disambiguates an options object (e.g., {y: "x2"}) from a primitive value.
export function isObject(option) {
return option && option.toString === objectToString;
return option?.toString === objectToString;
}

// Disambiguates a scale options object (e.g., {color: {type: "linear"}}) from
// some other option (e.g., {color: "red"}). When creating standalone legends,
// this is used to test whether a scale is defined; this should be consistent
// with inferScaleType when there are no channels associated with the scale, and
// if this returns true, then normalizeScale must return non-null.
export function isScaleOptions(option) {
return isObject(option) && (option.type !== undefined || option.domain !== undefined);
}

// Disambiguates an options object (e.g., {y: "x2"}) from a channel value
Expand Down
8 changes: 8 additions & 0 deletions test/legends/legends-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,11 @@ it("Plot.legend({color: {type:'identity'}}) returns undefined", () => {
const l = Plot.legend({color: {type: "identity"}});
assert.strictEqual(l, undefined);
});

it("Plot.legend({}) throws an error", () => {
assert.throws(() => Plot.legend({}), /unknown legend type/);
});

it("Plot.legend({color: {}}) throws an error", () => {
assert.throws(() => Plot.legend({color: {}}), /unknown legend type/);
});
17 changes: 17 additions & 0 deletions test/output/emptyLegend.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions test/plots/empty-legend.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import * as Plot from "@observablehq/plot";

export default async function() {
return Plot.plot({
color: {
legend: true // ignored because no color scale
},
marks: [
Plot.frame()
]
});
}
1 change: 1 addition & 0 deletions test/plots/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export {default as diamondsCaratPrice} from "./diamonds-carat-price.js";
export {default as diamondsCaratPriceDots} from "./diamonds-carat-price-dots.js";
export {default as documentationLinks} from "./documentation-links.js";
export {default as empty} from "./empty.js";
export {default as emptyLegend} from "./empty-legend.js";
export {default as emptyX} from "./empty-x.js";
export {default as figcaption} from "./figcaption.js";
export {default as figcaptionHtml} from "./figcaption-html.js";
Expand Down

0 comments on commit 36a873c

Please sign in to comment.