Skip to content

Conversation

@LeeLenaleee
Copy link
Collaborator

Resolves #10700

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Would it be sufficient to have the test in datasetController?

Also we could abstract the message, but that should be done if/when we get another similar warning.

@LeeLenaleee
Copy link
Collaborator Author

I thought only the dataset controller wasn't sufficient enough since it wouldn't throw an error when you set fill:true in the options.

Downside of this is that if you specify fill true in the dataset and root of the options it will throw the warning multiple times

@kurkle
Copy link
Member

kurkle commented Sep 24, 2022

The dataset options should resolve to the root options, if not specified at dataset level, so it should work that way. Maybe add tests? :)

@LeeLenaleee
Copy link
Collaborator Author

LeeLenaleee commented Sep 24, 2022

You are right 😅 , my 2AM testing brain was flawed so it seemed 😬

Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Couple of nits

@LeeLenaleee LeeLenaleee merged commit 5f37ba6 into chartjs:master Sep 26, 2022
@LeeLenaleee LeeLenaleee deleted the add-warning-on-using-filler-while-not-registering branch September 26, 2022 18:21
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.

Emit warning when fill option is specified but filler plugin is not registered

3 participants