Skip to content
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

beforeDraw hook #97

Open
samselikoff opened this issue Apr 28, 2014 · 7 comments
Open

beforeDraw hook #97

samselikoff opened this issue Apr 28, 2014 · 7 comments

Comments

@samselikoff
Copy link

It'd be nice to have a beforeDraw hook, a place to do chart-wide work not related to any particular layer (e.g. updating a scale's domain), but completely removed from the transform cascade.

Thoughts?

(I took a look at the library myself and was playing around with implementing, but figured I'd message here first.)

@jugglinmike
Copy link
Member

Tell me if I'm interpreting your request correctly:

transform beforeDraw
argument(s) chart data none
return value passed along to draw method of layers and attachments ignored
invocation all implementations on the prototype chain ("cascading") the first implementation on the prototype chain (typical JavaScript)

Is that right?

@samselikoff
Copy link
Author

Almost - I think beforeDraw should have the chart data as its argument, since it's related to drawing (and you draw with data, typically).

This way you could use the data to, for example, update your scale's domains.

@iros
Copy link
Member

iros commented Apr 28, 2014

Sam, that is the intent of the "transform" method. It takes in the data as
the only argument and is called before any of the layers'/attached charts'
draw methods. It must return the data (possibly transformed) because that
is what will be passed to the layers/attached charts, but otherwise you can
do with it as you wish before it exists. The transform method is only
called once.

I use it traditionally to update scale domains, create layouts etc.
Perhaps it's the method name that isn't appropriate for what it actually
does?

-- Irene

On Mon, Apr 28, 2014 at 11:02 AM, Sam Selikoff notifications@github.comwrote:

Almost - I think beforeDraw should have the chart data as its argument,
since it's related to drawing (and you draw with data, typically).

This way you could use the data to, for example, update your scale's
domains.


Reply to this email directly or view it on GitHubhttps://github.com//issues/97#issuecomment-41568925
.

@samselikoff
Copy link
Author

So there was a situation I ran into that prevented me from being able to use transform for this, and I think it was the cascade. Does a layer's or attached chart's transform method get called before or after the parent chart's?

I think I wanted a layer/attached chart's domain to override the parent's. When I have more time I'll go back and try to get what the exact use case was.

@iros
Copy link
Member

iros commented Apr 28, 2014

That would be incredibly helpful Sam! Let us know when there's an example
of that, that we can look at.

-- Irene

On Mon, Apr 28, 2014 at 11:38 AM, Sam Selikoff notifications@github.comwrote:

So there was a situation I ran into that prevented me from being able to
use transform for this, and I think it was the cascade. Does a layer's or
attached chart's transform method get called before or after the parent
chart's?

I think I wanted a layer/attached chart's domain to override the parent's.
When I have more time I'll go back and try to get what the exact use case
was.


Reply to this email directly or view it on GitHubhttps://github.com//issues/97#issuecomment-41573767
.

@samselikoff
Copy link
Author

@iros you can actually see the convo here: #78 (comment)

The problem was because transform is called upwards, so it doesn't behave as I would normally expect inheritance to (i.e. super is called last, instead of first).

But perhaps I need to rethink the hierarchy a la Mike's suggestion. Still, having a method that is reliably invoked last on a subchart could be useful.

@jugglinmike
Copy link
Member

Another solution would be to build support for constructors into d3.chart (see #76). If it were clear how to access any given chart constructor, then we could remove both initCascade and transformCascade and instead defer to the user for prototype traversal. This would facilitate @samselikoff's use case here, since he could call Parent.prototype.transform(data) and then perform some additional work.

Removing the "cascading" behavior would be a pretty significant breaking change, and I don't suggest it lightly. But it may be that the -cascade methods are too clever for their own good, so I want to at least put that on the table.

@samselikoff In the mean time, you should be able to solve your problem with the refactoring I suggested in that issue.

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

No branches or pull requests

3 participants