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

Add Padding method make distinct from Margin #35

Open
yanofsky opened this issue Mar 10, 2015 · 8 comments
Open

Add Padding method make distinct from Margin #35

yanofsky opened this issue Mar 10, 2015 · 8 comments

Comments

@yanofsky
Copy link
Contributor

apples-oranges_chartbuilder 77

We use margin to define the space between the edge of the chart and the next closest item: the credit line the legend, the axis etc.

We use padding to define the space between the margin and the chart area.

We also have a method of extraPadding so that we can add or subtract from the default padding without modifying it.

@heavysixer
Copy link
Owner

Is this some code you can share? I ran into a race condition when trying to get this to work originally. Essentially if the user specifies an outer width or height for the chart then the inner chart dimensions are determined by subtracting the margin from the outer values. If the padding is configured later then the margin values are now invalid and I don't have a mechanism inside d4 to invalidate those values based on a later change. If you've cracked that nut i'd be happy to integrate that change. Additionally, the larger issue of making certain variable changes dependent on other variables is something I need to solve in general especially as we get into more complex charting types.

@yanofsky
Copy link
Contributor Author

The way we implemented it leaves the chart size calculations untouched (for better or for worse)

We added the methods to the chart
https://github.com/yanofsky/d4/blob/master/src/base.js#L753-L771

then we changed the way scales are calculated https://github.com/yanofsky/d4/blob/master/src/builders/scales.js#L14-L20

(we also changed the way axes position
https://github.com/yanofsky/d4/blob/master/src/features/y-axis.js#L76-L85
but we've customized d3 to draw axes differently)

@heavysixer
Copy link
Owner

@yanofsky I've been giving this some thought and I think the way to handle this is to add a reactive programming component to base which will allow you to assign data dependencies to variables. For example if the inner width of the chart area is effected by the padding and margin then any change to those should cause a recalculation. Something like this:

d4.when(['padding', 'margin'], function (padding, margin) {
  // recalculate the available visual space for the chart.
});

This proposed approach is based off of @curran's model library https://github.com/curran/model

@yanofsky
Copy link
Contributor Author

That looks like a good solution, but shouldn't it be applied to chart not d4?.

It would also be convenient if you could update a single property of margin/padding using this method e.g.

chart.when(['padding', 'margin'], function (padding, margin) {
  // recalculate the available visual space for the chart.
});

chart.when("paddingLeft", function (paddingLeft) {
  // recalculate the available visual space for the chart.
});

chart.set({
  margin: {top:10, right: 10, bottom: 10, left: 10},
  padding: {top:0, right: 0, bottom: 0, left: 0}
});

chart.set({
  paddingLeft: 10
})

console.log(chart.padding())
// {top:0, right: 0, bottom: 0, left: 10}

@heavysixer
Copy link
Owner

@yanofsky agreed, my thought was that it would happen transparently inside base when a user used one of the accessors like chart.width(100) or chart.marginLeft(20). This would trigger an event listener inside the base itself to recalculate all the dependent properties. This keeps the API consistent with what it is today.

With that said there is nothing to say we could not expose a when() function at the chart level to allow users to hook in their own dependencies where they so choose e.g.

chart.when(['data'], function(data){});

@yanofsky
Copy link
Contributor Author

awesome!

@heavysixer
Copy link
Owner

yeah now i just need to write it ;-)

@curran
Copy link

curran commented Mar 12, 2015

Here's an example that might help
http://bl.ocks.org/curran/015d34d6d3d562877e51 .

Also, here's "when" formulated as an extension to Backbone (called "wire"
in this example)
https://github.com/curran/phd/blob/dac07e2e8c38da7343645d7a07ec17a762120ea0/prototype/src/wire.js
.

Good luck with this work, looking forward to seeing how it goes.

On Thu, Mar 12, 2015 at 8:00 AM, Mark Daggett notifications@github.com
wrote:

yeah now i just need to write it ;-)


Reply to this email directly or view it on GitHub
#35 (comment).

yanofsky referenced this issue in yanofsky/d4 Aug 20, 2015
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