Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Wrong category size of Bar with min ,max options. #3589

Closed
KoyoSE opened this issue Nov 13, 2016 · 8 comments · Fixed by #3648
Closed

[BUG] Wrong category size of Bar with min ,max options. #3589

KoyoSE opened this issue Nov 13, 2016 · 8 comments · Fixed by #3648

Comments

@KoyoSE
Copy link
Contributor

KoyoSE commented Nov 13, 2016

Hi! thank you for provide excellent chart library.
I would like to report some issue.

Current Behavior

With min (or max) option, Category size probably calculated from the total number of labels, not the number of labels displayed.
Then, the size of bar will be smaller and shift ,even though categoryPercentage and barPercentage is 1.
(also Issues #3506 plugin added)

report-chartjs-wrongcategorysize

Steps to Reproduce (for bugs)

jsfiddle

  1. Chart type is "bar"
  2. Set min or max, both also.
  3. When executed, the bar position will be smaller and shift left. (You can see easily when categoryPercentage and barPercentage is 1 )
  4. line is work.

Environment

Chart.js version : 2.4.0
Browser name and version : Microsoft Edge 38.14393.0.0
Link to my project : https://jsfiddle.net/d29yoL8u/5
Added Issues #3506 duplicate of #3491 and #2870

@Jareechang
Copy link
Contributor

@etimberg, I can fix this one.

I have a question in regards to ticks.min option, in the documentation it only shows to allow Number. Will we be changing that to allow string datatypes (ex. 'March') ?

screen shot 2016-11-14 at 1 03 40 pm

@etimberg
Copy link
Member

@Jareechang feel free to work on this 😄 Please include test(s) with your fix.

I think ticks.min can be 'March' for the category axis (the one in question here)

@Jareechang
Copy link
Contributor

ok, sounds good! i'll do that 👍

@KoyoSE
Copy link
Contributor Author

KoyoSE commented Nov 17, 2016

Probably, I think this problem can be solved with this correction.

Possible Solution

Delete the following two codes.

https://github.com/chartjs/Chart.js/blob/master/src/controllers/controller.bar.js#L142-L145

https://github.com/chartjs/Chart.js/blob/master/src/controllers/controller.bar.js#L469-L472

With this fix, it becomes as follows.

2016-11-18
2016-11-18 2

jsfiddle

Environment

Chart.js version : 2.4.0
Browser name and version : Microsoft Edge 38.14393.0.0
Link to my project : https://jsfiddle.net/2eq6jxL7/

(I have poor English. so I am sorry if I am rude.)

@etimberg
Copy link
Member

@KoyoSE that's a great fix. Could you PR it and add some tests?

@Jareechang
Copy link
Contributor

nice solution @KoyoSE.

I was playing around with the code as well. I came up with this solution code section, we can just add if min or max defined then it'll center the bars. You can maybe try this one if it starts breaking the api.

Another issue

I found another bug while playing with the jsfiddle. The tooltip label is incorrect when min is defined.

Should I open another issue for this ?

screen shot 2016-11-17 at 12 33 39 pm

@etimberg
Copy link
Member

@Jareechang please open another issue for the tooltip issue

@KoyoSE
Copy link
Contributor Author

KoyoSE commented Nov 18, 2016

Hi @etimberg
I will try adding PR and Test. However, I am still a beginner in JavaScript development. And I recently started using Github. Please give me some time. :)

Hi @Jareechang
Oh I also checked the code section.
but.If min or max defined in that section,
Probably it have to write processing to recalculate the mistaken fullBarWidth in getRuler function.

KoyoSE added a commit to KoyoSE/Chart.js that referenced this issue Nov 27, 2016
for Bar and horizontalBar type,
include stacked scale.
issue chartjs#3589
@etimberg etimberg added this to the Version 2.5 milestone Dec 2, 2016
etimberg pushed a commit that referenced this issue Dec 2, 2016
for Bar and horizontalBar type,
include stacked scale.
issue #3589
exwm pushed a commit to exwm/Chart.js that referenced this issue Apr 30, 2021
for Bar and horizontalBar type,
include stacked scale.
issue chartjs#3589
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants