interval for rect #550
Merged
interval for rect #550
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I guess this should probably set default insets, too, like e.g. bin does? |
Fil
reviewed
Sep 24, 2021
|
|
||
| function maybeInset(inset, inset1, inset2) { | ||
| return inset === undefined && inset1 === undefined && inset2 === undefined | ||
| ? (offset ? [1, 0] : [0.5, 0.5]) |
There was a problem hiding this comment.
(This is not new) The default inset makes the marks vanish if they are too narrow. For example in aapl-volume-rect, if you take the whole dataset instead of just a slice, the resulting chart is blank.
There was a problem hiding this comment.
refs. #422 and d3/d3-scale#243
There was a problem hiding this comment.
I’m aware of this problem but not going to fix it here. 👍
|
I don’t think we should expose the interval (or inset) transform yet, out of conservatism, but we can always chose to do so in the future if we think it’s useful. |
|
I think I want to add the interval transform to bar and rule, too. |
mbostock
force-pushed
the
mbostock/rect-interval
branch
from
56d3da8 to
cb26d3f
Compare
|
Added. Works nicely! |
Fil
approved these changes
Sep 24, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds an interval option for rects which transforms x to x1 and x2 using interval.floor and interval.offset. This provides a suitable replacement for bars when an axis is temporal rather than ordinal. Related #513, and perhaps this could be considered a fix.
I’d further love to be able to specify a named interval such as “day” or “hour”, but this requires knowing the time zone (or for now, whether the associated scale is “utc” or local “time”). That’s not easily accessible when evaluating a transform since this happens before scales are constructed. Although I suspect that with a top-level timeZone option #531 we could find some way of passing this down to the transform. At any rate, convenience shorthand can be added in the future.