Skip to content

Call resize handlers only if chart is displayed#2164

Merged
kt3k merged 1 commit intoc3js:masterfrom
upphiminn:master
Sep 15, 2017
Merged

Call resize handlers only if chart is displayed#2164
kt3k merged 1 commit intoc3js:masterfrom
upphiminn:master

Conversation

@upphiminn
Copy link
Copy Markdown

@upphiminn upphiminn commented Sep 5, 2017

In the current implementation the resize handlers are called even if the chart is not displayed. This is not ideal when having one chart in focus and some hidden ones, as all event handlers get called and performance degrades. Also, while Chrome behaves correctly and does not actually change the width of the hidden ones, Firefox tries to resize the hidden elements and sets incorrect full widths, visible after the hidden charts are re-displayed. By checking the offsetParent we can tell if the chart is displayed or not and we can skip resize handling if hidden.

@kt3k

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #2164 into master will decrease coverage by 0.09%.
The diff coverage is 20%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2164     +/-   ##
=========================================
- Coverage   73.98%   73.88%   -0.1%     
=========================================
  Files          51       51             
  Lines        4178     4185      +7     
=========================================
+ Hits         3091     3092      +1     
- Misses       1087     1093      +6
Impacted Files Coverage Δ
src/core.js 89.19% <20%> (-0.93%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edc746e...c0a40a8. Read the comment docs.

@kt3k
Copy link
Copy Markdown
Member

kt3k commented Sep 8, 2017

Thanks for the contribution. This change makes sense to me.

offsetParent seems to be null as well when the element itself has position: fixed style, but I think nobody use position: fixed directly to the chart's element. (possibly somebody would use position: fixed for the parent of the chart, but that doesn't cause a problem.)

@kt3k kt3k merged commit 7e5c20c into c3js:master Sep 15, 2017
@kt3k kt3k mentioned this pull request Feb 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants