Refactor plot autosize

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.

[etpinard]

fixes#537

This PR sets up a base branch for the in-progress autosize refactor starting out by @n-riesco in #577 patched in #629 and then reverted in #633.

Important comments:

• autosize: Fix Plots.resize backwards-compatibility #629 (comment)
• Remove internal option layout.autosize='initial' (Fixes #537) #577 (comment)

[mdtusz]

Just for my own understanding: when would this not be an HTMLElement?

[etpinard]

if someone calls Plotly.Plots.supplyDefaults({ data: [], layout: {}}); - which should be allowed.

[n-riesco]

If gd isn't a HTMLElement, el3.classed('js-plotly-plot') throws an exception.

[mdtusz]

💃

[etpinard]

dropping this PR from the v1.17.0 milestone because of the numerous merge conflict left to fix ... and let's face it, we're due for a minor bump.

[fresheneesz]

@etpinard

I created some logic that finds the maximum size an element can be sized to without exceeding the boundaries of its parent. It takes into account margins, borders, padding, percentage widths, max-height/width, and sibling nodes. You can find it here: http://stackoverflow.com/a/39782139/122422

I think this is something that would help autosizing work better, since currently using autosizing seems to frequently exceed the boundaries of the graph's parent.

[etpinard]

Dropping from the v1.18.0 milestone.

This will be implemented in v1.19.0 together with layout breakpoints.

[sarendipitee]

I'm super interested in this PR. Is there anything I can contribute?

I am trying to use Plotly in a Flex layout and Plotly is not playing nicely. The major problem is Plotly's use of explicit width/height in pixels. Doing this makes an implicit size on the chart that will then cause the flex layout to never go less than size.

Here's an example pen: http://codepen.io/JonDum/pen/gwEPPA

If you resize horizontally you'll notice that it will grow larger, but never smaller.

If you look at how Chartist.js handles this, they default to setting the <svg> element to a width/height of 100% and they automatically scale to their container's size (which you can see on their demo page)

[etpinard]

@n-riesco I (finally) resurrected your PR 🎉

To preserve backward compatibility, I had to add two commits:

• 77971f4 adds a hacky block in relayout to make sure this relayout case still passes. It's not pretty, but only a few lines. I hope you don't mind.
• b6c5610 is a little trickier and very important. This commit ensures that graphs with no set width and height have a default autosize of true. Previously, graphs with no set width and height were given the 'initial' autosize value (see here).

I hope you find these two commits compatible with what you had in mind for this PR.

[n-riesco]

Looks like github didn't get my email. Here's a copy:

---

On 26/10/16 18:50, Étienne Tétreault-Pinard wrote:

• 77971f4

I don't understand how this commit works.

If I read it correctly, something like Plotly.relayout(gd, { width: null }) would be equivalent to Plotly.relayout(gd, { width: 'initial' }).

Since 'initial' is an invalid width, this would make gd._fullLayout.width = 700 (i.e. the default width value).

Would the following work instead?

if(['width','height'].indexOf(ai)!==-1&&vi===null){deletegd._fullLayout._initialAutoSizeIsDone;}

I can look into this tomorrow.

---

• b6c5610

I think this behaviour is better (and necessary to fix #537)

[etpinard]

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

I think this behaviour is better

By this, you meant behaviour after my last commit (b6c5610) ?

(and necessary to fix #537)

The tests you added to fix #537 are still passing, anything else I should try to confirm that #537 is still fixed?

[etpinard]

@n-riesco

Would the following work instead?

if(['width', 'height'].indexOf(ai) !== -1 && vi === null) {
delete gd._fullLayout._initialAutoSizeIsDone;
}

Let me try. Thanks!

Replacing 77971f4 with your suggestion makes the aforementioned relayout test case fail.

The reason being:

• clearing _fullLayout._initialAutoSizeDone leads to a plotAutosize call which (in the default context) reaches this line where the new width/height are taken from the DOM
• The problem is: those width/height value come from the previous relayout call, so line by line:

it('sets null values to their default',function(done){vardefaultWidth;Plotly.plot(gd,[{x: [1,2,3],y: [1,2,3]}]).then(function(){defaultWidth=gd._fullLayout.width;defaultWidth// => 700returnPlotly.relayout(gd,{width: defaultWidth-25});}).then(function(){gd._fullLayout.width// => 675expect(gd._fullLayout.width).toBe(defaultWidth-25);returnPlotly.relayout(gd,{width: null});}).then(function(){gd._fullLayout.width// => 675 BUT should 700 !!expect(gd._fullLayout.width).toBe(defaultWidth);}).then(done);});

[etpinard]

Now, by setting width: 'initial' in relayout in the hacky block of 77971f4, Plots.supplyDefaults now reaches this line, where newLayout.width isn't reset to its old value as 'initial' is truthy. Instead, newLayout.width is set back to its default value of 700.

[etpinard]

Perhaps a better solution would be to cache the initial graph div width / height computed styles and use them in relayout.

[etpinard]

@n-riesco here's a less hacky (I think) fix: 8396018

Let me know what you think.

[etpinard]

@n-riesco commit 08069a4 implements an even less hacky solution (I think we now have the correct answer to what relayout should do).

Let me know what you think.

[etpinard]

@n-riesco I took your 👍 reaction on #635 (comment) as a sign of approval. Commit 5fb7cd1 implement the cache-initial-autosize solution.