Skip to content

responsive handler: do not resize if gd is hidden#3972

Merged
antoinerg merged 3 commits into
masterfrom
fix-3866
Jun 25, 2019
Merged

responsive handler: do not resize if gd is hidden#3972
antoinerg merged 3 commits into
masterfrom
fix-3866

Conversation

@antoinerg

Copy link
Copy Markdown
Contributor

Fixes #3866 using approach number 2 from comment #3866 (comment)

Comment thread src/plot_api/plot_api.js Outdated
function isHidden(gd) {
var display = window.getComputedStyle(gd).display;
return !display || display === 'none';
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the same helpers as in Plots.resize?

Can we make this a Lib function?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good idea!

Done in 913bb68

Comment thread src/plot_api/plot_api.js Outdated
if(!gd._responsiveChartHandler) {
// Keep a reference to the resize handler to purge it down the road
gd._responsiveChartHandler = function() { Plots.resize(gd); };
gd._responsiveChartHandler = function() { if(!isHidden(gd)) Plots.resize(gd).catch(Lib.noop); };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really need to .catch here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't! The whole point is to not ignore possible rejected promises. Thanks, @etpinard for noticing this.

Fixed in 913bb68

@etpinard etpinard added status: reviewable bug something broken labels Jun 18, 2019
@etpinard etpinard added this to the v1.49.0 milestone Jun 18, 2019
@etpinard

Copy link
Copy Markdown
Contributor

Thanks for taking this on @antoinerg !

Pinning this for v1.49.0

@etpinard

Copy link
Copy Markdown
Contributor

thanks again for doing this @antoinerg 💃

@etpinard

Copy link
Copy Markdown
Contributor

(let's start merging things for 1.49.0)

@antoinerg
antoinerg merged commit 705b184 into master Jun 25, 2019
@antoinerg
antoinerg deleted the fix-3866 branch June 25, 2019 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug something broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Resize on hidden graph divs causes Error

2 participants