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

Sort line and bubble dataset options alphabetically #10100

Merged
merged 2 commits into from Feb 1, 2022
Merged

Sort line and bubble dataset options alphabetically #10100

merged 2 commits into from Feb 1, 2022

Conversation

stockiNail
Copy link
Contributor

This PR is sorting alphabetically the line and bubble dataset options in the documentation

LeeLenaleee
LeeLenaleee previously approved these changes Jan 26, 2022
etimberg
etimberg previously approved these changes Jan 26, 2022
@stockiNail
Copy link
Contributor Author

I have a doubt.

The drawActiveElementsOnTop option has been documented in the bubble and line datasets and in styling section.

But, being in the styling section, the fallback should be to elements.point, as documented:

All these values, if undefined, fallback to the associated elements.point.* options.

Nevertheless I think that drawActiveElementsOnTop option doesn't have any fallback to elements.point, therefore it should be placed in the General section.

Correct me if I'm wrong.

@LeeLenaleee
Copy link
Collaborator

LeeLenaleee commented Jan 26, 2022

No I think you are correct, at the time of making this I saw it as a styling thing since it changes the way things look but it cant be configured per bubble as the docs suggest and it doesn't listen to options.elements.point.drawActiveElementsOnTop for a fallback.

So it should indeed be moved to the general section for the extra explanation.

@stockiNail
Copy link
Contributor Author

Fine! Let me apply the change and to re-commit for further approval.

@stockiNail stockiNail dismissed stale reviews from etimberg and LeeLenaleee via 3a5d985 January 26, 2022 17:54
@stockiNail
Copy link
Contributor Author

@LeeLenaleee FYI I'm not a collaborator in Chart.js project therefore I can not merge it by myself! ;)

@LeeLenaleee
Copy link
Collaborator

Me neither :)

Only have triage access, which means I can add milestones, labels to issues and pr's but I don't have write access so I can't merge, edit create or delete.

Only people I know that have write access are etimburg, kurkle, benmcan, nagix and simonbrunel.

@etimberg etimberg merged commit 56661e4 into chartjs:master Feb 1, 2022
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.

None yet

4 participants