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.

Sign up for GitHub

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

Jump to bottom

fix: Add backgroundColor type on CoreScaleOptions #11348

Merged
merged 3 commits into from
Aug 7, 2023
Merged

fix: Add backgroundColor type on CoreScaleOptions #11348

merged 3 commits into from
Aug 7, 2023

Conversation

Mer-cat
Copy link
Contributor

@Mer-cat Mer-cat commented Jun 13, 2023 •
edited
Loading

Very similar to issue #11082 with same fix, but in CartesianScaleOptions added to CoreScaleOptions so this isn't duplicated in various extensions of the CoreScaleOptions

@Mer-cat Mer-cat marked this pull request as ready for review June 13, 2023 23:15
@Mer-cat Mer-cat changed the title Fix: add backgroundColor type on CartesianScaleOptions fix: Add backgroundColor type on CartesianScaleOptions Jun 13, 2023
@LeeLenaleee LeeLenaleee added the type: types Typescript type changes label Jun 14, 2023
@LeeLenaleee LeeLenaleee added this to the Version 4.3.1 milestone Jun 14, 2023
LeeLenaleee
LeeLenaleee requested changes Jun 14, 2023
View reviewed changes
Copy link
Collaborator

@LeeLenaleee LeeLenaleee left a comment

Choose a reason for hiding this comment

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

Since it applies to all scales it would be better to move it to the CoreScaleOptions so it is not duplicate

@Mer-cat Mer-cat changed the title fix: Add backgroundColor type on CartesianScaleOptions fix: Add backgroundColor type on CoreScaleOptions Jun 14, 2023
@Mer-cat Mer-cat requested a review from LeeLenaleee June 14, 2023 16:20
@Mer-cat
Copy link
Contributor Author

Mer-cat commented Jul 18, 2023

Can you let me know if there is anything to further change here? Would love to have the corrected type merged :) Thanks!

@LeeLenaleee
Copy link
Collaborator

MB for the late response, since its now in the core scale opts it can be removed from the RadialLinearScaleOptions at line 3436. After that it should be fine

@Mer-cat Mer-cat marked this pull request as draft July 25, 2023 20:08
@Mer-cat Mer-cat force-pushed the mer-cat/fix-background-color-cartesianscale branch from b1c542b to c85c9f6 Compare July 25, 2023 20:14
@Mer-cat Mer-cat force-pushed the mer-cat/fix-background-color-cartesianscale branch from c85c9f6 to 32c226e Compare July 25, 2023 20:17
@Mer-cat Mer-cat marked this pull request as ready for review July 25, 2023 20:17
@Mer-cat
Copy link
Contributor Author

Mer-cat commented Aug 2, 2023 •
edited
Loading

I don't have merge permissions, so if you would like to merge this please do. Thanks for your approvals :)

@etimberg
Copy link
Member

etimberg commented Aug 2, 2023

@Mer-cat we'll merge once we release a 4.3.3 to fix a bug

@LeeLenaleee LeeLenaleee merged commit a3fc841 into chartjs:master Aug 7, 2023
9 checks passed
This was referenced May 26, 2024
Merged
Merged
@raymondhtet raymondhtet mentioned this pull request May 29, 2024
Closed
@brmzkw brmzkw mentioned this pull request Jun 7, 2024
Open
@OKEAMAH OKEAMAH mentioned this pull request Jun 8, 2024
Open
@wanissr wanissr mentioned this pull request Jun 22, 2024
Open
@bwilliford bwilliford mentioned this pull request Jun 26, 2024
Open
@raymondhtet raymondhtet mentioned this pull request Jul 5, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etimberg etimberg etimberg approved these changes

@LeeLenaleee LeeLenaleee LeeLenaleee approved these changes

Assignees
No one assigned
Labels
type: types Typescript type changes
Projects
None yet
Milestone
Version 4.4.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@Mer-cat @LeeLenaleee @etimberg