Skip to content

Use scale defined in the chart when x/y ScaleID options are not set #678

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

Merged
merged 7 commits into from
Apr 4, 2022
Merged

Use scale defined in the chart when x/y ScaleID options are not set #678

merged 7 commits into from
Apr 4, 2022

Conversation

stockiNail
Copy link
Collaborator

@stockiNail stockiNail commented Feb 10, 2022

Fix #676
This PR removes the x/yScaleID options defaults from annotation. It will use the scale (defined as x or y) in the chart and only when 1 scale for the coordinate is defined.

TODO

  • documentation update

@stockiNail stockiNail added this to the 2.0.0 milestone Feb 10, 2022
@stockiNail stockiNail marked this pull request as ready for review February 10, 2022 12:39
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.

Looks good overall, just couple of comments / questions,

@stockiNail
Copy link
Collaborator Author

Looks good overall, just couple of comments / questions,

If we will start going to V2 and due to element normalization, maybe it could be helpful to define the sequence of PR merging in order to help the rebasing, where needed. My 2 cents.

@kurkle
Copy link
Member

kurkle commented Mar 17, 2022

Looks good overall, just couple of comments / questions,

If we will start going to V2 and due to element normalization, maybe it could be helpful to define the sequence of PR merging in order to help the rebasing, where needed. My 2 cents.

I think that would be your task, because you've done all the work. So maybe just keep things as draft and mark the next one ready after merge.

@stockiNail
Copy link
Collaborator Author

stockiNail commented Mar 17, 2022

I think that would be your task, because you've done all the work. So maybe just keep things as draft and mark the next one ready after merge.

Fine for me. In fact, they are all in Draft, apart of the V2 migration page one which must be the first one.

kurkle
kurkle previously approved these changes Mar 17, 2022
@stockiNail stockiNail requested a review from kurkle April 4, 2022 06:51
@stockiNail stockiNail merged commit 6bf1a72 into chartjs:master Apr 4, 2022
@stockiNail stockiNail deleted the noDefaultScales branch April 4, 2022 07:25
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.

Line is diagonal if yAxis options set
2 participants