Skip to content

Conversation

@mbostock
Copy link
Member

Alternative to #618. Fixes #616. This behavior is more consistent with how rects treat insets, e.g.,

plot/src/marks/rect.js

Lines 57 to 60 in 72b2adb

.attr("x", X1 && X2 && !isCollapsed(x) ? i => Math.min(X1[i], X2[i]) + insetLeft : marginLeft + insetLeft)
.attr("y", Y1 && Y2 && !isCollapsed(y) ? i => Math.min(Y1[i], Y2[i]) + insetTop : marginTop + insetTop)
.attr("width", X1 && X2 && !isCollapsed(x) ? i => Math.max(0, Math.abs(X2[i] - X1[i]) - insetLeft - insetRight) : width - marginRight - marginLeft - insetRight - insetLeft)
.attr("height", Y1 && Y2 && !isCollapsed(y) ? i => Math.max(0, Math.abs(Y1[i] - Y2[i]) - insetTop - insetBottom) : height - marginTop - marginBottom - insetTop - insetBottom)

I’d slightly prefer using the midpoint here, something like:

if (right >= left) {
  scale.range = [left, right];
} else {
  const center = (right + left) / 2;
  scale.range = [center, center];
}

But then I’d want rects to behave similarly, and I don’t think it’s really worth the trouble for such a rare thing. Mostly I just want to avoid the surprising (to me) behavior of the scale’s range getting inverted when the insets or margins are too big, and I think a collapsed range is a sufficient indication of a misconfiguration and we don’t need to do more.

@mbostock mbostock requested a review from Fil December 31, 2021 17:35
@mbostock mbostock merged commit 390aabe into main Jan 1, 2022
@mbostock mbostock deleted the mbostock/disallow-range-inversion branch January 1, 2022 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Insets should not allow the range to invert

2 participants