Skip to content

Conversation

Mer-cat
Copy link
Contributor

@Mer-cat Mer-cat commented Jun 13, 2023

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
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 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

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

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

Labels

type: types Typescript type changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants