Chart sets height automatically if none is specified

[ghost]

I wanted the containing SVG element to shrink to fit the actual content in my timeline. Since I'm using the timeline to display an arbitrary number of recurring events, I can't predict ahead of time how tall the timeline will be. So I thought it would make sense to have the height property automatically set itself to the height of the timeline's content after the timeline gets called. This allowed me to then set the SVG element's "height" attribute to the timeline's height after calling the timeline on it.

I wonder if it would make more sense to set the SVG element's height from the timeline, though, since it seems at present that setting "height" doesn't actually do anything to the timeline's height.

[jiahuang]

display an arbitrary number of recurring events

There's no way to get how many series of data you're displaying before putting it in the timeline?

But, you're right, height on the timeline doesn't do anything at its present state.

I wonder if it would make more sense to set the SVG element's height from the timeline, though, since it seems at present that setting "height" doesn't actually do anything to the timeline's height.

That could work. But then "width" wouldn't do the same thing due to the scrolling feature. So that'd be weird.

Another alternative is to proportionally scale the height of the rendered SVG elements according to the overall height. So if it has 3 series of data that is unstacked and a height of 100, each series would get a height of 30px with 5px in between for padding. But that kinda screws with itemHeight and doesn't really work for your scenario.

Your proposed way of doing it seems like the best out of all these choices. I'll merge it unless you want to discuss some more alternative ways. Lemme know.

Thanks for writing this up btw :)

[ghost]

No problem!

Yes, I can get how many data series I'm working with ahead of time and I can then calculate the height outside of d3-timeline. But I saw that we had this "height" function that returned a height and that we weren't doing anything with it, so I figured it'd make more sense to make the calculation part of the library.

The inconsistency with the behavior of width does bother me too. Maybe width should automatically be set to the length of the timeline. That way the behavior would at least be consistent.

And should these two things be functions at all? They seem more like properties. Did you have a use case in mind for setting the width and the height?

Thanks!

[jiahuang]

Maybe width should automatically be set to the length of the timeline. That way the behavior would at least be consistent.

Then .scroll() could would take in a callback function and a resize width? That could work.

And should these two things be functions at all? They seem more like properties. Did you have a use case in mind for setting the width and the height?

Yeeeaaah. Originally it was actually supposed to set the SVG height and width, but then for some reason I completely messed that up.

Getting rid of height and width and making use of the SVG's height/width seems like the way that makes the most sense here. Thoughts?

[ghost]

Then .scroll() could would take in a callback function and a resize width? That could work.

I'm not sure I like using .scroll() to resize. What if a person just wants to resize the width? They shouldn't have to define a callback function. And using .scroll() to resize seems a little counterintuitive, to be honest.

Getting rid of height and width and making use of the SVG's height/width seems like the way that makes the most sense here.

I do want to at least be able to get the timeline content's height so I can shrink the SVG element to fit. I was thinking that height and width could just be immutable object properties that reflect the timeline's content dimensions. (Which, I guess, would be done by making them functions that take no parameters.)

What do you think?

[jiahuang]

What if a person just wants to resize the width?

I'm not seeing the use case here. If someone wants dynamic resizing, then the entire graph will have to get re-rendered anyway.

And using .scroll() to resize seems a little counterintuitive, to be honest.

I agree.

immutable object properties that reflect the timeline's content dimensions

Width actually needs to get set in order to render the timeline (or are you thinking of setting it based off of the parent SVG's dimensions?). Height just doesn't due to the existing itemHeight property.

I'm assuming that what we want are:

1. ability to set the timeline's default width
2. ability to set the timeline's default height
3. ability to change the rendered width so that you are able to scroll
4. ability to change the rendered height (probably for y-axis scrolling if ever implemented)

In 1 & 2 the SVG height/width should be the same as the timeline's height/width. In 3 & 4 the rendered height/width only needs to change because of the scrolling.

So you're in scenario 2 but the SVG height doesn't match. I think that the cleanest way to handle this would be to:

1. get rid of itemHeight, height, and width
2. make height and width the same as the SVG element.
3. make itemHeight based off of the overall height proposed in https://github.com/jiahuang/d3-timeline/pull/7#issuecomment-14699870
4. make .scroll take in re-rendering height/width parameters

While I agree that .scroll() shouldn't be exclusively used to resize, I don't know when else you would need to resize (or render only a part of a timeline) unless you are scrolling. .scroll() would probably also have to get broken out into scrollX() and scrollY()

But maybe my assumptions are totally wrong here and I'm missing use cases?

[ghost]

I'm not seeing the use case here.

To be fair, I wouldn't personally find it useful either. Mostly I was going for consistency. Like what if someone wants a timeline that's no wider than the timeline's content, as a figure floated left of an article (for example)? That would require getting the timeline's content width.

Width actually needs to get set in order to render the timeline

Ah, right. Sorry, I forgot that part. So the user would already know the timeline's content width, right? Thus making a function to retrieve the timeline's width a bit redundant.

I'm starting to understand that we've got two different mental models surrounding how height and width gets set and how the bars and spaces between ticks get sized, both of which are mixed together in our current implementation. It seems that you're arguing we should have the height and width explicitly sized and the bars and spaces between ticks implicitly sized. That would give us greater control over the absolute size of the SVG element and (if the timeline is scrolling) the timeline's content size, at the expense of having control over the spaces between the ticks and the sizes of the bars themselves. In my particular use case, I actually prefer the opposite: explicit control over the bar heights and tick spacing while letting scrolling make up for any discrepancies between the content size and the SVG element's size. (In fact, I'm currently setting the width of my timeline by subtracting the beginning timestamp from the ending timestamp and then dividing by 1000 to give me the tick spacing I want.)

So getting rid of itemHeight doesn't really help me in my case since I do need to be able to specify the heights of the individual bars in order to create enough room for the icons I'm using. I could figure out what height I need to set the SVG element to in order to get the proper height for each of the bars (sort of like I do with width and tick spacing), but that seems like a bit of a PITA compared to what we have now. I would prefer to just set the itemHeight based on the size of the bars/icons I want to use and then set the SVG element's height based on the timeline's content height.

While I agree that .scroll() shouldn't be exclusively used to resize, I don't know when else you would need to resize (or render only a part of a timeline) unless you are scrolling. .scroll() would probably also have to get broken out into scrollX() and scrollY()

True enough. If we do go with explicitly setting SVG width and height and implicitly setting the content formatting, then that approach does totally make sense.

[jiahuang]

Ah, thanks for clearing that up.

I actually prefer the opposite: explicit control over the bar heights and tick spacing while letting scrolling make up for any discrepancies between the content size and the SVG element's size.

Alright, that makes sense; a lot of preprocessing is really annoying to do when the timeline does it anyway. I'll pull in this change now.

Sorry this took so long. I just wanted to make sure I understood what was going on design-wise. Thanks for this.

[ghost]

No worries. It's good to talk through these sorts of decisions. :-)