-
Notifications
You must be signed in to change notification settings - Fork 4
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
Make colors generator public to allow for customized colors for chart #26
Conversation
@@ -46,7 +46,7 @@ class Chart extends View | |||
protected $datasets; | |||
|
|||
/** @var ColorGenerator */ | |||
protected $colorGenerator; | |||
public $colorGenerator; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can be set using DI (seed)
@@ -16,7 +16,7 @@ class ColorGenerator | |||
* | |||
* @var array<int, array<int, string>> | |||
*/ | |||
protected $colors = [ | |||
public $colors = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there is a setter method for it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point :-)
@DarkSide666 did you read the comments? |
Oh, I got Covid and somehow missed them. Would you like to revert this? Looks like it's useless if we use DI and setter method instead. |
Beyond having an automatied ColorGenerator, it needs to be possible to set the colour scheme per chart easily. Thus, both ColorGenerator and its $color definition should be public.