Skip to content

pass the "exposed scales" to the render function #1265

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

Closed
wants to merge 2 commits into from
Closed

Conversation

Fil
Copy link
Contributor

@Fil Fil commented Feb 9, 2023

This cleans up the render API (#1263), as the render function now receives the same scale object as is exposed through the plot.scale("x") API.

It means a bit of extra work, as axes have to compute their own tickFormat and ticks, and a special case for the x and y identity scales. But it's worth it, in the name of consistency.

The difference that I still need to reduce is the scale’s label, which we censor when exposing the scale… I think this could change (we could expose it)… alternatively, we could try and pass the label through a separate argument.

todo

  • decide whether we expose the scale's label or censor it

It would be a breaking change for, e.g., https://observablehq.com/@observablehq/plot-calendar, which reads x.bandwidth() and would need to use x.bandwidth instead (number).

@Fil Fil marked this pull request as draft February 9, 2023 16:57
@Fil Fil requested a review from mbostock February 9, 2023 16:57
@Fil Fil force-pushed the fil/pass-scales branch from 8ef2184 to 5691e2e Compare March 29, 2023 21:02
@Fil
Copy link
Contributor Author

Fil commented Mar 29, 2023

rebased, and retyped

@Fil Fil marked this pull request as ready for review March 29, 2023 21:06
@Fil
Copy link
Contributor Author

Fil commented Aug 23, 2023

superseded by #1810

@Fil Fil closed this Aug 23, 2023
@Fil Fil deleted the fil/pass-scales branch August 23, 2023 07:49
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.

1 participant