Float-bar support. The logic is to use array values [bottomY, topY] i… #5262

gwyneblaidd · 2018-02-11T02:50:14Z

Float-bar support. The logic is to use array values [bottomY, topY] instead of regular vals. SO if you use regular numbers chart will be not floated and bar start will be 0 by default. If you use array for value current update allowing to track this and use bottomY value from array instead of 0.)

Example:
http://pravopys.net/chartjs/samples/charts/bar/vertical.html

…nstead of regular vals. SO if you use regular numbers char will be not floated and bottomY will be 0 by default. If you use array for value current update allowing to track this and use bottomY value from array instead of 0.)

src/controllers/controller.bar.js

src/core/core.scale.js

benmccann · 2018-02-11T03:11:10Z

Thanks for this! This will close numerous issues linked to in #4863

docs/charts/bar.md

src/scales/scale.linear.js

gwyneblaidd · 2018-02-13T17:57:02Z

Hi @simonbrunel, @etimberg!

This pull request for adding floating bar support. Can you check pls, so maybe it should be upgraded/changed.

This should fix #4863 issue.

simonbrunel · 2018-02-14T09:00:47Z

I think it's a good start, I need to review it more deeply, but don't have time right now. I agree with the data format [min, max] (as suggested in #4863), but we need to be sure we are picking the right one because if it's merged, we can't change it until next major version.

We also need to be sure that:

it works with horizontal bars
it works with stacked bars

A few early feedback:

instead of duplicating the logic to pick the "right" value in each scale, it may be better to move it directly in getRightValue()? Though, that would need some investigation to be sure it doesn't break anything.
I would not introduce the new borderSkipped: 'none' value, but instead support borderSkipped: false|null.
I would not call it lowY or highY but low/hight or min/max
we will need unit tests

…ys for Y val)

A few early feedback: instead of duplicating the logic to pick the "right" value in each scale, it may be better to move it directly in getRightValue()? Though, that would need some investigation to be sure it doesn't break anything. I would not introduce the new borderSkipped: 'none' value, but instead support borderSkipped: false|null. I would not call it lowY or highY but low/hight or min/max

gwyneblaidd · 2018-02-24T23:49:22Z

Hi @simonbrunel @benmccann !

I have made changes according to your suggestions.

I have tested all chars, so here the links for you to test it also:

vertical char with linear scale
http://pravopys.net/chartjs/samples/charts/bar/vertical.html
vertical char with log scale
http://pravopys.net/chartjs/samples/charts/bar/vertical-log.html
vertical-stacked char with linear scale
http://pravopys.net/chartjs/samples/charts/bar/stacked-vertical.html
vertical-stacked char with log scale
http://pravopys.net/chartjs/samples/charts/bar/stacked-vertical-log.html

horizontal char with linear scale
http://pravopys.net/chartjs/samples/charts/bar/horizontal.html
horizontal char with log scale
http://pravopys.net/chartjs/samples/charts/bar/horizontal-log.html
horizontal -stacked char with linear scale
http://pravopys.net/chartjs/samples/charts/bar/stacked-horizontal.html
horizontal -stacked char with log scale
http://pravopys.net/chartjs/samples/charts/bar/stacked-horizontal-log.html

simonbrunel · 2018-03-04T10:26:07Z

There are multiple approaches to handle {min, max} stacked bars:

a. ignore stacking (which seems the solution adopted in this PR )
b. stack height only (start at 0):
- bar(0).y0 = 0
- bar(0).y1 = bar(0).max - bar(0).min
- bar(n).y0 = bar(n-1).y1
- bar(n).y1 = bar(n).y0 + (bar(n).max - bar(n).min)
c. stack height (start at bar(0).min):
- bar(0).y0 = bar(0).min
- bar(n).y0 = bar(n-1).y1
- bar(n).y1 = bar(n).y0 + (bar(n).max - bar(n).min)
d. stack from origin:
- bar(0).y0 = bar(0).min
- bar(n).y0 = bar(n-1).y1 + bar(n).min
- bar(n).y1 = bar(n).y0 + (bar(n).max - bar(n).min)

a. can be achieved without stacking
b. can be achieved by stacking number data
c. implies no gap between bars but doesn't necessary start at origin
d. implies gap between bars

I don't think a. is correct because it's not "stacked" but then I'm not sure between b, c and d. Maybe d. is the best behavior (that's what @etimberg suggested in #4863) because it allows to implement c. as well by mixing {x, y} and number data.

master_up

benmccann · 2019-01-16T20:23:11Z

@gwyneblaidd the rebase seems not to have worked. It looks like most of your changes disappeared

gwyneblaidd · 2019-01-16T20:54:59Z

@gwyneblaidd the rebase seems not to have worked. It looks like most of your changes disappeared

yepp, i had problems with rebasing, everytime smth was in conflict, i had to create new pull request to this one to replace all files so now this pull request has no changes regarding to original repo. But i will return back today all my changes with new commits.

benmccann · 2019-02-08T03:19:10Z

src/platforms/platform.dom.js

@@ -124,11 +124,11 @@ var supportsEventListenerOptions = (function() {
 // https://github.com/chartjs/Chart.js/issues/4287
 var eventListenerOptions = supportsEventListenerOptions ? {passive: true} : false;

-function addEventListener(node, type, listener) {
+function addListener(node, type, listener) {


Not sure why cbb7ff7 is showing up on this PR. Seems the rebase didn't totally work

when i has all that failing tests i thought that smth really bad went with merge last time. So, i downloaded lates Chart.js files, replaced with it all my local files and commited to my fork. So, in this case my repo should look like the current Chart.js version. So, all that changes came when i replaced files. I have put back my changes again on float-bat support after. Are thise chanages on this file are not correct?

i have replaced all files from current chart.js version. Is this file is not correct?

i think because this was commited only 2 days ago thats why i picked it

The changes shouldn't appear on your PR. The problem is that your PR is comparing to a version of the code from a few days ago probably instead of to the latest master. You should run git rebase master again (and maybe make a backup copy first before doing that)

It's probably really hard to rebase with 82 commits spanning a year on the PR. It's possible you might have an easier time opening a new PR at this point rather than rebasing

~~... or you can squash all your 82 commits in a single one, and rebase it on top of master. Then we can keep working in the same PR with all previous comments.~~

@gwyneblaidd if you need help squashing / rebasing this PR, please join our Slack #dev channel. But please don't open a new PR with these 82 commits, it will be useless.

Forget my comment about squashing commits, I didn't realize you already merged many times our master into your branch, which makes the operation quite complicated, so it would be easier to start a new PR from scratch.

rebased PR - #6056

royduin · 2019-02-08T12:31:21Z

Just for reference, new PR: #6056

benmccann reviewed Feb 11, 2018

View reviewed changes

src/controllers/controller.bar.js Outdated Show resolved Hide resolved

benmccann reviewed Feb 11, 2018

View reviewed changes

src/core/core.scale.js Outdated Show resolved Hide resolved

benmccann added the type: enhancement label Feb 11, 2018

gwyneblaidd added 5 commits February 11, 2018 05:18

formatting

ceafcaf

formatting&fix

f2e7310

formatting

d07f183

formatting

4ae0988

formatting&docs

5b5bd25

benmccann reviewed Feb 11, 2018

View reviewed changes

docs/charts/bar.md Outdated Show resolved Hide resolved

benmccann reviewed Feb 11, 2018

View reviewed changes

src/scales/scale.linear.js Outdated Show resolved Hide resolved

benmccann reviewed Feb 11, 2018

View reviewed changes

src/scales/scale.linear.js Outdated Show resolved Hide resolved

gwyneblaidd added 2 commits February 11, 2018 19:38

logarithmic scale is added

88aa1a5

Update bar.md

fc478a6

gwyneblaidd added 11 commits February 14, 2018 14:03

core scale hardcoding for getRightBarValue(). Float bar support (arra…

6075e6f

…ys for Y val)

Merge remote-tracking branch 'origin/master'

85ea4da

latest changes with stacked bar linera/log scale and float0bar support

87e6e38

formatting

0e55557

tests

0dd7666

formatting

8e44c5e

formatting

70a11e5

formatting

362d5f9

formatting

12525ce

formatting

4a366a2

gwyneblaidd dismissed benmccann’s stale review via b626b57 January 16, 2019 18:47

gwyneblaidd added 2 commits January 16, 2019 21:35

Merge branch 'master' into master_up

b25db90

Merge pull request #1 from gwyneblaidd/master_up

8f06bc5

master_up

gwyneblaidd added 13 commits January 17, 2019 12:51

Update controller.bar.js

2a07aaa

Update core.scale.js

d165e24

Update core.tooltip.js

c4148e9

Update element.rectangle.js

a48b8ce

Update scale.linear.js

e1095fa

Update scale.logarithmic.js

14bccfe

Back changes after rebase

f4d7413

formatting

5590350

formatting

9f88e05

fixes

76e9ceb

Merge branch 'master' into master

f957e64

formatting

2d06063

Merge branch 'master' of https://github.com/gwyneblaidd/Chart.js

a786cd1

benmccann mentioned this pull request Feb 6, 2019

Allow configuration of borderWidth as object #6047

Closed

gwyneblaidd added 7 commits February 8, 2019 01:40

fallback to test units

74dc634

border skipped added

dba5bdb

_parseValue added. Scales updated

0e58967

format

124688c

+

84dae3d

controller bar update

19a400c

format

87b0d91

benmccann reviewed Feb 8, 2019

View reviewed changes

simonbrunel closed this Feb 8, 2019

gwyneblaidd mentioned this pull request Feb 9, 2019

Add support for floating bar chart ([start, end]) #6056

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Float-bar support. The logic is to use array values [bottomY, topY] i… #5262

Float-bar support. The logic is to use array values [bottomY, topY] i… #5262

gwyneblaidd commented Feb 11, 2018 •

edited

Loading

benmccann commented Feb 11, 2018

gwyneblaidd commented Feb 13, 2018

simonbrunel commented Feb 14, 2018

gwyneblaidd commented Feb 24, 2018 •

edited

Loading

simonbrunel commented Mar 4, 2018

benmccann commented Jan 16, 2019

gwyneblaidd commented Jan 16, 2019

benmccann Feb 8, 2019

gwyneblaidd Feb 8, 2019

gwyneblaidd Feb 8, 2019

gwyneblaidd Feb 8, 2019

benmccann Feb 8, 2019

simonbrunel Feb 8, 2019 •

edited

Loading

simonbrunel Feb 8, 2019

simonbrunel Feb 8, 2019

gwyneblaidd Feb 8, 2019

royduin commented Feb 8, 2019

Float-bar support. The logic is to use array values [bottomY, topY] i… #5262

Float-bar support. The logic is to use array values [bottomY, topY] i… #5262

Conversation

gwyneblaidd commented Feb 11, 2018 • edited Loading

benmccann commented Feb 11, 2018

gwyneblaidd commented Feb 13, 2018

simonbrunel commented Feb 14, 2018

gwyneblaidd commented Feb 24, 2018 • edited Loading

simonbrunel commented Mar 4, 2018

benmccann commented Jan 16, 2019

gwyneblaidd commented Jan 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonbrunel Feb 8, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

royduin commented Feb 8, 2019

gwyneblaidd commented Feb 11, 2018 •

edited

Loading

gwyneblaidd commented Feb 24, 2018 •

edited

Loading

simonbrunel Feb 8, 2019 •

edited

Loading