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

fix: color errors due to fillColor changes in v54.0.0 #7271

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

nickofthyme
Copy link
Contributor

Summary

This PR fixes errors in partition chart coloring due to fillColor api changes in v54.0.0 (from elastic/elastic-charts#1956).

We missed this change in updating chart in #6641 with 6 breaking changes. Also we have become accustom to typescript flagging errors like this but all errors in this case were in js files 😭.

This changed the API where the first param of the fillColor function is the Key of the layer and not the entire Node. The second param is now the sortIndex used in many places in the EUI docs.

- export type NodeColorAccessor = (d: ShapeTreeNode, index: number, array: HierarchyOfArrays) => string;
+ export type NodeColorAccessor = (key: Key, sortIndex: number, node: ArrayNode, tree: HierarchyOfArrays) => string

@nickofthyme nickofthyme added the documentation Issues or PRs that only affect documentation - will not need changelog entries label Oct 9, 2023
@nickofthyme nickofthyme requested a review from a team as a code owner October 9, 2023 19:26
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

@nickofthyme nickofthyme enabled auto-merge (squash) October 9, 2023 20:12
Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

❤️ Thanks for grabbing this Nick! I can confirm the console error is gone in staging.

Also we have become accustom to typescript flagging errors like this but all errors in this case were in js files 😭.

We can totally change the charts docs pages to TS if you're down for that - totally up to you, of course!

@nickofthyme nickofthyme merged commit f2cb634 into elastic:main Oct 9, 2023
10 of 12 checks passed
@nickofthyme nickofthyme deleted the fix-pie-coloring branch October 10, 2023 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants