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

Added option to calculate height and/or width on render. #327

Closed
wants to merge 1 commit into from

Conversation

matthull
Copy link
Contributor

By default, uses height and width of anchor DOM element.
Can optionally specify function to use for calculation.

Doesn't address all the auto-sizing features listed in issue #295, but I'm finding it very useful for these scenarios:

  • Have chart fill parent div, thus controlling chart size via CSS
  • Have chart dynamically resize in response to user interactions (e.g. user changes top N count with slider)

By default, uses height and width of anchor DOM element.
Can optionally specify function to use for calculation.
@gordonwoodhull
Copy link
Contributor

I don't know if Nick and Jacob would find this too clever, but I'd suggest allowing the width and height methods to take either a function or a value, in the d3 / crossfilter style.

@matthull
Copy link
Contributor Author

I was leaning that way at first, but wasn't sure if it would be too much magic. Definitely keeps the API cleaner though - I felt a little dirty adding 4 new API methods just for this feature.

Thoughts, Nick/Jacob? Would something like the following be copasetic?
.height(50) // no change in behavior
.height(function(element) {...}) // set height calculation
.height("auto") // enables default height calculation - element.getBoundingRect().height or similar

@jrideout jrideout mentioned this pull request Oct 10, 2013
@jrideout
Copy link
Contributor

@matthull I like the approach you and @gordonwoodhull are thinking about. I implemented a version in #329. Let me know if you think that works. I didn't put in an "auto" option, but rather will "revert to default" on any falsy value to achieve the same thing. I find that a bit cleaner, but that's just my personal preference.

@gordonwoodhull Anything we can do to keep API style similar to crossfilter and d3 is a must. Our abstraction is purposefully very leaky, so we need environmental, not internal consistency to maintain the principle of least surprise. Plus d3.functor is cool.

@ghost ghost assigned jrideout Oct 10, 2013
@matthull
Copy link
Contributor Author

I like it. To me the semantics of falsy triggering default are a little weird, but I think it's cleaner and easier to remember vs. using magic word like "auto."

@matthull matthull closed this Oct 10, 2013
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.

3 participants