Prevent negative width or height on rect

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663

[vdh]

When working with responsive container sizes for charts, I've encountered Error: <rect> attribute height: A negative value is not valid. DOM errors (Chrome 65.0.3325.162). Because the container area is sometimes too small for a reasonable chart, the _length calculations sometimes go into the negatives.

This PR is just a simple guard to avoid setting invalid negative values on the rect elements.

[etpinard]

Thanks very much for bringing this up! I'm not convinced your patch is the best way to fix this bug though. _length shouldn't be negative in the first place when reaching this part of the code.

Would you mind sharing a reproducible example that exhibits the DOM errors you're referring to?

[vdh]

@etpinard
I was able to recreate it in CodePen ( https://codepen.io/vdh/pen/WzpOoe ), although it may be connected to react-plotly.js's useResizeHandler option (or the Plotly.Plots.resize call it wraps…?).

[vdh]

I've managed to make a smaller demo https://codepen.io/vdh/pen/OvpjmX
For some reason useResizeHandler / Plotly.Plots.resize is causing it, but I haven't been able to dig deeper yet as it would require reverse-engineering the code of react-plotly.js to recreate the Plotly calls.

[n-riesco]

@vdh I see the same issue with react-chart-editor (see my comment here). In my case, an easy way to trigger this issue is to set display: none in the container node and trigger a resize.

[alexcjohnson]

I'm assuming what's happening here is the margins are larger than the plot area - ie margin.l + margin.r > width, either because width is very small or we can't determine it at all (invisible node cc @n-riesco ). These situations are going to arise from time to time, and we shouldn't barf of course... if width or height is <=0 we should bail out of plotting entirely (and early), and if the margins are too big to display anything... dunno what makes most sense here, either shrink the margins so we still have plotting space, or just display the title and bail?

[vdh]

I also discovered two additional places where rects are given negative widths for tiny charts (and added guards to the PR):

• drawing.setRect / drawing.setSize in src/components/drawing/index.js
• axes.makeClipPaths in src/plots/cartesian/axes.js

@etpinard@alexcjohnson
I know these guards don't directly address the code that generates the negative dimensions, but is it really so bad to guard against the generation of invalid SVG attributes? These DOM errors create a lot of noise in the console.

[etpinard]

I also discovered two additional places where rects are given negative widths for tiny charts

Right. This is exactly why we're hoping to find the root cause of this problem.

Instead of patching every piece of code that might write negative svg width/height, we should (at least try to) make sure _length and other positive definite fields don't become negative in the first place.

[etpinard]

... to do so, we'll need some sort of reproducible example in plotly.js.

[grahambryan]

Has there been any resolution to this problem? I have been running into the same problem within react-plotly.js within my react app.

Error: attribute width: A negative value is not valid. ("-50") @ plotly.js:17171

[vdh]

@etpinard

... to do so, we'll need some sort of reproducible example in plotly.js.

So are you implying that the CodePen examples I provided earlier are inadequate? Is there a better way to create an example? I apologise that I don't have the skills or familiarity with Plotly to separate react-plotly.js from the examples, but on Chrome 66.0.3359.139 I can reproduce the same console errors using those CodePens.

[etpinard]

@vdh as far as I know no one has been able to reproduce this issue using only plotly.js (i.e. no react, no react-plotly.js). This doesn't mean plotly.js isn't doing something wrong here. But it would help you pin down why theses values are negative in the first place.

[etpinard]

Ok. I was able to reproduce this bug in pure plotly.js.

See https://codepen.io/etpinard/pen/KRjKpg?editors=0010 and open the console to see the logs.

It seems to be a problem with Plots.resize (which is the plotly.js method called when react-plotly.js' useResizeHandler option is turned on).

Closing this PR and moving the discussion to #2663