Skip to content

Conversation

@missingdays
Copy link

@missingdays missingdays commented Jul 5, 2016

Add new chart.width() and chart.height() API methods for making resize easier.
This closes #1752

@codecov-io
Copy link

codecov-io commented Jul 5, 2016

Codecov Report

Merging #1753 into master will increase coverage by 0.21%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1753      +/-   ##
==========================================
+ Coverage   77.12%   77.33%   +0.21%     
==========================================
  Files          51       51              
  Lines        4131     4143      +12     
==========================================
+ Hits         3186     3204      +18     
+ Misses        945      939       -6
Impacted Files Coverage Δ
src/api.chart.js 56.75% <100%> (+44.75%) ⬆️

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 c45b30f...f85c3d9. Read the comment docs.

@pshulga-dev
Copy link

How to use for resize chart?

@missingdays
Copy link
Author

@PashaShulga width() and height() are both setters and getters
So chart.width(someNumber) will resize chart to someNumber width. The same goes for height.
If you need to get current width, you just call chart.width().

If data is grouped:
  - the tooltip will keep the same ordering as the stacked values

If data is not grouped:
  - the tooltip will use the data_order option to sort the values

 Also adds an optional 'tooltip_order' option. If set, it will override the data_order option.
@SlyTheKing
Copy link

This would be extremely helpful 👍

@panthony
Copy link
Contributor

@missingdays I'd argue that maybe a size() getter/setter that takes { height: X, width: X } would be better ?

For the a lib that is already struggling with performance reason, providing an API that allows updating width and height at the same time and do a single flush may be better.

What do you think?

@missingdays
Copy link
Author

@panthony there is already API for resizing using both width and height
https://github.com/c3js/c3/blob/master/src/api.chart.js#L3

Separate width and height API makes it easier to change one property without getting the other. So instead of

chart.resize({'width': 500, 'height': chart.config.size.height})

you could just write

chart.width(500)

@panthony
Copy link
Contributor

@missingdays resize() should clearly handle resize({width: 100}) or resize({height: 50}).

And in this case width(100) could simply be an alias of resize({ width: 100 }).

@missingdays
Copy link
Author

missingdays commented Apr 14, 2018

@panthony I might be wrong here, but if you don't pass the other property in resize, it is set to null, which means "auto-size", as getCurrentWidth here calls $$.getParrentWidth if the property is set to null.

@panthony
Copy link
Contributor

panthony commented Apr 15, 2018

@missingdays You're right, what I had in mind would change the current behavior which is not good.

Well, if you could just update the documentation to add height() and width() to the documented API.

The documentation site is now part of this repository.

@missingdays missingdays changed the base branch from dev to master April 18, 2018 18:47
@missingdays
Copy link
Author

Added docs for width and height to reference.html.haml

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.

chart.width() and chart.height() API

6 participants