Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

only identity if all values are colors #676

Merged
merged 2 commits into from
Jan 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 27 additions & 5 deletions src/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,6 @@ export const first = d => d[0];
export const second = d => d[1];
export const constant = x => () => x;

// A few extra color keywords not known to d3-color.
const colors = new Set(["currentColor", "none"]);

// Some channels may allow a string constant to be specified; to differentiate
// string constants (e.g., "red") from named fields (e.g., "date"), this
// function tests whether the given value is a CSS color string and returns a
Expand All @@ -37,7 +34,7 @@ const colors = new Set(["currentColor", "none"]);
export function maybeColorChannel(value, defaultValue) {
if (value === undefined) value = defaultValue;
return value === null ? [undefined, "none"]
: typeof value === "string" && (colors.has(value) || color(value)) ? [undefined, value]
: isColor(value) ? [undefined, value]
: [value, undefined];
}

Expand Down Expand Up @@ -213,10 +210,35 @@ export function isNumeric(values) {
export function isColors(values) {
for (const value of values) {
if (value == null) continue;
return typeof value === "string" && (colors.has(value) || color(value) !== null);
return isColor(value);
}
}

// Whereas isColors only tests the first defined value and returns undefined for
// an empty array, this tests all defined values and only returns true if all of
// them are valid colors. It also returns true for an empty array, and thus
// should generally be used in conjunction with isColors.
export function isAllColors(values) {
for (const value of values) {
if (value == null) continue;
if (!isColor(value)) return false;
}
return true;
}

// Mostly relies on d3-color, with a few extra color keywords. Currently this
// strictly requires that the value be a string; we might want to apply string
// coercion here, though note that d3-color instances would need to support
// valueOf to work correctly with InternMap.
export function isColor(value) {
if (!(typeof value === "string")) return false;
value = value.toLowerCase();
return value === "currentcolor" || value === "none" || color(value) !== null;
}

// Like a sort comparator, returns a positive value if the given array of values
// is in ascending order, a negative value if the values are in descending
// order. Assumes monotonicity; only tests the first and last values.
export function order(values) {
if (values == null) return;
const first = values[0];
Expand Down
8 changes: 5 additions & 3 deletions src/scales.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {parse as isoParse} from "isoformat";
import {isColors, isOrdinal, isTemporal, order} from "./options.js";
import {isAllColors, isColors, isOrdinal, isTemporal, order} from "./options.js";
import {registry, color, position, radius, opacity, symbol, length} from "./scales/index.js";
import {ScaleLinear, ScaleSqrt, ScalePow, ScaleLog, ScaleSymlog, ScaleQuantile, ScaleThreshold, ScaleIdentity} from "./scales/quantitative.js";
import {ScaleDiverging, ScaleDivergingSqrt, ScaleDivergingPow, ScaleDivergingLog, ScaleDivergingSymlog} from "./scales/diverging.js";
Expand Down Expand Up @@ -202,8 +202,10 @@ function inferScaleType(key, channels, {type, domain, range}) {
if (registry.get(key) === symbol) return "ordinal";
for (const {type} of channels) if (type !== undefined) return type;
if (registry.get(key) === color
&& (domain !== undefined ? isColors(domain)
: channels.some(({value}) => value !== undefined && isColors(value)))) return "identity";
&& (domain !== undefined
? isColors(domain) && isAllColors(domain)
: channels.some(({value}) => value !== undefined && isColors(value))
&& channels.every(({value}) => value === undefined || isAllColors(value)))) return "identity";
if ((domain || range || []).length > 2) return asOrdinalType(key);
if (domain !== undefined) {
if (isOrdinal(domain)) return asOrdinalType(key);
Expand Down