Skip to content

Remove unnecessary checks for hidden data#6255

Closed
benmccann wants to merge 1 commit into
chartjs:masterfrom
benmccann:hidden
Closed

Remove unnecessary checks for hidden data#6255
benmccann wants to merge 1 commit into
chartjs:masterfrom
benmccann:hidden

Conversation

@benmccann

@benmccann benmccann commented May 7, 2019

Copy link
Copy Markdown
Contributor

Hiding of an individual data item is undocumented and no controller using these scales actually supports it.

This leaves support for hidden for the polarArea and doughnut controllers and radialLinear scale

@kurkle

kurkle commented May 8, 2019

Copy link
Copy Markdown
Member

It comes from #2346.
Not sure why this does not break any tests.

@kurkle kurkle requested a review from simonbrunel May 8, 2019 14:34
@benmccann

Copy link
Copy Markdown
Contributor Author

I would be surprised if it broke any tests since the controllers don't use the hidden attribute. The functionality seems unrelated to the goal of #2346 so I'm not sure the original intention of adding it

@simonbrunel simonbrunel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Element supports the hidden state (public) and controllers should handle it since it can be altered by external code (custom legend, plugins, etc.). This PR would likely break a few integrations.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants