Skip to content

render API #501

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
Fil opened this issue Aug 14, 2021 · 6 comments
Closed

render API #501

Fil opened this issue Aug 14, 2021 · 6 comments
Labels
documentation Improvements or additions to docs question Further information is needed

Comments

@Fil
Copy link
Contributor

Fil commented Aug 14, 2021

#498 has introduced function marks: () => svg node

That render function is called like any other and receives as input (data, scales, channels, dimensions), with empty data and channels, all the instantiated scales, and complete dimensions.

It would be useful I think to design and document this API (for all marks). The changes I'd suggest would be:

  1. pass the facet's "content" (i.e. "Chinstrap")
  2. order the API as (dimensions, scales, facet, data, channels)
  3. change the dimensions in such a way that we can ignore margins (ie width would be the current width-marginLeft-marginRight, and the would already be translated by marginLeft, marginTop)

https://observablehq.com/d/cfdce446a293784f

@mbostock mbostock added documentation Improvements or additions to docs question Further information is needed labels Aug 14, 2021
@mbostock
Copy link
Member

We can’t change this API without breaking backwards compatibility, so I do not wish to redesign it. I agree we should document mark.render though.

@Fil
Copy link
Contributor Author

Fil commented Aug 16, 2021

Would you be against passing facet as a fifth argument?

@mbostock
Copy link
Member

I realized we are inconsistent with what we pass mark.render. Without faceting, we pass {index, scales, values, dimensions, axes}; with faceting we pass {index, scales, values, dimensions}. So, if we want to pass the current facet it would need to be a sixth argument, or we’d somehow need to squirrel it into one of the existing arguments. But aside from how I’m onboard with somehow exposing the current facet values to marks during rendering.

@Fil
Copy link
Contributor Author

Fil commented Aug 18, 2021

One of the recurring frustrations with the current API is that render doesn't know who the parent SVG and the parent FIGURE will be.

This makes it awkward to build interactivity, since we have to wait (asynchronously) for the g we return to be appended to the plot before we can bind listeners, or know where to dispatch our custom events to update the viewof.

It's still probably too early to cast this in concrete, but it should be included in the discussion about the API. Having the 6th argument be {facet} would allow to extend it in the future to {facet, parentSVG, figure} or something else (a callback when the figure is added to the DOM, maybe).

@Fil Fil mentioned this issue Aug 19, 2021
3 tasks
@Fil
Copy link
Contributor Author

Fil commented Aug 19, 2021

I did a bit of documentation in https://observablehq.com/d/cfdce446a293784f ; but def. not a blocker for 0.2

@Fil Fil mentioned this issue Aug 20, 2021
@Fil Fil mentioned this issue Nov 23, 2021
@Fil Fil mentioned this issue Jan 25, 2022
This was referenced Dec 22, 2022
@Fil
Copy link
Contributor Author

Fil commented Feb 8, 2023

superseded by #1263

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to docs question Further information is needed
Projects
None yet
Development

No branches or pull requests

2 participants