Add support for projections to the standalone Plot.scale function#2427
Add support for projections to the standalone Plot.scale function#2427
Conversation
_e.g._ Plot.scale({projection: {type: "mercator", width, domain: …}}).
docs/features/scales.md
Outdated
| const plot2 = Plot.plot({...options2, color: plot1.scale("color")}); | ||
| ``` | ||
|
|
||
| Plot.scale also supports projections. <VersionBadge version="0.6.18" /> The returned projection object exposes *apply* and *invert* methods for converting between geographic and pixel coordinates, and can be passed as the **projection** option of another plot. |
There was a problem hiding this comment.
| Plot.scale also supports projections. <VersionBadge version="0.6.18" /> The returned projection object exposes *apply* and *invert* methods for converting between geographic and pixel coordinates, and can be passed as the **projection** option of another plot. | |
| Plot.scale also supports projections. <VersionBadge pr="2427" /> The returned projection object exposes *apply* and *invert* methods for converting between geographic and pixel coordinates, and can be passed as the **projection** option of another plot. |
Let’s not get ahead of ourselves. 🙂
| @@ -673,3 +674,4 @@ export interface Scale extends ScaleOptions { | |||
| * ``` | |||
| */ | |||
| export function scale(options?: {[name in ScaleName]?: ScaleOptions}): Scale; | |||
There was a problem hiding this comment.
Is options really optional here?
| export function scale(options?: {[name in ScaleName]?: ScaleOptions}): Scale; | |
| export function scale(options: {[name in ScaleName]?: ScaleOptions}): Scale; |
| return scale; | ||
| } | ||
|
|
||
| function scaleProjection({ |
There was a problem hiding this comment.
Should this be called exposeProjection for symmetry?
There was a problem hiding this comment.
Yes. There is already an exposeProjection function but it could be renamed to prepareProjection.
| marginLeft = margin, | ||
| ...projection | ||
| }) { | ||
| if (height === undefined) height = width * projectionAspectRatio(projection); |
There was a problem hiding this comment.
This doesn’t look like what we do in autoHeight…
There was a problem hiding this comment.
True. I'll make a helper function to be used in both places.
mbostock
left a comment
There was a problem hiding this comment.
As far as testing goes, I’m hoping that we can test this new implementation against the existing (more wasteful) one. This:
Plot.plot({projection: {type: "mercator"}}).scale("projection")Should be the same as this:
Plot.scale({projection: {type: "mercator"}})| * ``` | ||
| */ | ||
| export function scale(options?: {[name in ScaleName]?: ScaleOptions}): Scale; | ||
| export function scale(options: {projection: ProjectionOptions & {width?: number; height?: number}}): Projection; |
There was a problem hiding this comment.
This looks like it needs margin options, too. And probably aspectRatio? I guess those should be pulled out of PlotOptions and into something we can reuse here, maybe call it DimensionOptions?
Co-authored-by: Mike Bostock <mbostock@gmail.com>
e.g. Plot.scale({projection: {type: "mercator", width, domain: …}}).
When merged, we must add this to the changelog PR #2426: