height/contentHeight accept function for dynamic value

[aharpervc]

this makes it a lot easier to size the control to its container. for example, implementations could do return container_el.clientHeight

[aharpervc]

This is basically what I'm doing to solve #3039

[flick36]

+1

[arshaw]

i agree that #2704 should still be implemented, but this could be cool as well. I'd prefer to see it as the existing height or contentHeight option that simply accepts a function

[arshaw]

Philosophical question: how is this better than calling the height setter method?

I'd rather keep the setter method as the only way to dynamically set an explicit pixel value, and then introduce a height: 'parent' option as described in #2704

[aharpervc]

It's a more elegant way to handle a dynamically changing container.

[arshaw]

@aharpervc can you provide hypothetical code samples to show how it's more elegant the existing technique of using the setter methods?

[aharpervc]

I'll give you my actual production code I'm running with this patch:

this.timeline = $(container_el).fullCalendar(_.extend({
    timezone: "utc",
    editable: false,
    aspectRatio: 1.8,

    getContainerHeight: function() {
        return container_el.clientHeight - 15;
    },
    // etc...

There's no extra state to track and no extra events to hook up. Whenever calcSize is called internally, the current container client height is used. Now fullcalendar can vertically fill its container both on init and when the browser is resized. There's a - 15 there because in my actual app, there's kind of a margin, but I didn't want to invent an extra container div to put fullcalendar inside of just for that.

Now, you could certainly re-implement this concept as a static property. However, you'd need to make assumptions on what is the appropriate container element (probably direct parent) and what is the appropriate margins (probably none). Those assumptions may or may not be true for all apps and isn't relevant to the fullcalendar itself. That's why it's more elegant to delegate the computation to the caller.

[arshaw]

ah ok, this is more clear to me now.

how is the height-update initiated? are you manually calling the render method (which readjusts sizing) or are you merely waiting for the built-in window resize handler?

[aharpervc]

I'm not explicitly calling render, but also I do have handleWindowResize turned on.

[arshaw]

ok, this feature makes more sense to me now that i know you are leveraging window resize handling to adjust the height. thanks.

[arshaw]

could you leverage the else if chain instead of doing a return?

[arshaw]

would you be able to write tests for this feature? mandatory to ensure it doesn't break accidentally in the future.

https://github.com/fullcalendar/fullcalendar/wiki/Automated-Tests
https://github.com/fullcalendar/fullcalendar/tree/master/tests/automated

please let me know if i can give you any help/advice.

[aharpervc]

I may be able to revisit this at some point, but I don't currently have your test environment set up, or much capacity to investigate it at the moment.