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

Make colors generator public to allow for customized colors for chart #26

Merged
merged 1 commit into from
Dec 20, 2022

Conversation

mkrecek234
Copy link
Contributor

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.

@@ -46,7 +46,7 @@ class Chart extends View
protected $datasets;

/** @var ColorGenerator */
protected $colorGenerator;
public $colorGenerator;
Copy link
Member

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 = [
Copy link
Member

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 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point :-)

@DarkSide666 DarkSide666 merged commit dac0754 into develop Dec 20, 2022
@DarkSide666 DarkSide666 deleted the fix_colors branch December 20, 2022 09:37
@mvorisek
Copy link
Member

@DarkSide666 did you read the comments?

@DarkSide666
Copy link
Member

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.

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

Successfully merging this pull request may close these issues.

3 participants