Skip to content

Conversation

mathiask88
Copy link
Contributor

@etimberg I added some point styles like in Excel:
image

I know tests are failing, but I wanted to know what do you think of this, before changing the tests.

@etimberg
Copy link
Member

@mathiask88 I like the idea! Would make a great addition to v2.

@mathiask88
Copy link
Contributor Author

Added tests and rebased.

etimberg added a commit that referenced this pull request Jan 16, 2016
@etimberg etimberg merged commit 2db334d into chartjs:v2.0-dev Jan 16, 2016
@etimberg
Copy link
Member

Looks good. Thanks @mathiask88

@mathiask88
Copy link
Contributor Author

Cool, for the sake of completeness here an example of how it's looking:
image

Do you know why the last triangle looks different? :S

@etimberg
Copy link
Member

Looks like it might not stroke properly.

@Raf2k
Copy link

Raf2k commented Jan 18, 2016

Hi.

How I can add my own point styles?

@mathiask88
Copy link
Contributor Author

@Raf2k Just look at the changes made in element.point.js

@etimberg Looks like the colors get mixed up somewhere. It's always a color of another dataset and if there is only one dataset everything looks good.

@etimberg
Copy link
Member

@mathiask88 I didn't notice any colour issues. Do you have a jsfiddle?

@Raf2k
Copy link

Raf2k commented Jan 19, 2016

@mathiask88 I see that I should replace method draw to add my own point styles. (I don't want to hack original lib) Is this what you mean? Maybe something like this:

Chart.elements.Point.styles.myOwnStyleName = function() {...}

will be better? And draw function will handle this styles?

@mathiask88
Copy link
Contributor Author

@Raf2k Something like this? https://github.com/mathiask88/Chart.js/commit/8636ae2fc888ef0f5e8cc500595d5af680b8b6ed
With that fix you can assign a custom HTMLCanvasElement or HTMLImageElement to the pointStyle option that is drawn instead of the built in styles. (Not tested)

Edit: updated link

@mathiask88 mathiask88 deleted the pointStyles branch February 7, 2016 18:42
@mathiask88
Copy link
Contributor Author

@etimberg What do you think of the mentioned addition? Something the majority would appreciate?

@etimberg
Copy link
Member

@mathiask88 I think it would make a good addition :)

@mathiask88
Copy link
Contributor Author

#2031

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants