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(partition): rendering with small radius #2273

Merged
merged 4 commits into from
Dec 7, 2023
Merged

fix(partition): rendering with small radius #2273

merged 4 commits into from
Dec 7, 2023

Conversation

markov00
Copy link
Member

@markov00 markov00 commented Dec 6, 2023

Summary

This fix avoids throwing an error when rendering the chart due to a small partition chart.
The error is thrown by canvas when trying to render negative radius values. The fix blocks the geometry calculation before propagating this number in canvas functions.

Issues

Fix #2272

Checklist

  • The proper chart type label has been added (e.g. :xy, :partition)
  • The proper feature labels have been added (e.g. :interactions, :axis)
  • All related issues have been linked (i.e. closes #123, fixes #123)
  • Unit tests have been added or updated to match the most common scenarios

@markov00 markov00 added bug Something isn't working :data Data/series/scales related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels Dec 6, 2023
@markov00 markov00 mentioned this pull request Dec 6, 2023
Closed
@markov00
Copy link
Member Author

markov00 commented Dec 6, 2023

buildkite update screenshots

@markov00 markov00 marked this pull request as ready for review December 7, 2023 09:34
nickofthyme
nickofthyme approved these changes Dec 7, 2023
View reviewed changes
Copy link
Collaborator

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

Code changes, LGTM. Tested locally with test dimensions.

@markov00 markov00 merged commit 95a8537 into elastic:main Dec 7, 2023
13 checks passed
@markov00 markov00 deleted the 2023_12_06-fix_partition_not_rendering branch December 7, 2023 15:56
nickofthyme pushed a commit that referenced this pull request Dec 19, 2023
@semantic-release-bot
chore(release): 61.2.0 [skip ci]
dc705ad 
# [61.2.0](v61.1.0...v61.2.0) (2023-12-19)

### Bug Fixes

* **deps:** update dependency @elastic/eui to ^91.1.0 ([#2267](#2267)) ([308e974](308e974))
* **deps:** update dependency @elastic/eui to ^91.2.0 ([#2268](#2268)) ([29cdcb3](29cdcb3))
* **metric:** background colors and sparkline rendering ([#2255](#2255)) ([5abddfc](5abddfc))
* **partition:** rendering with small radius ([#2273](#2273)) ([95a8537](95a8537))
* **partition:** zero value sectors cause max stack call ([#2260](#2260)) ([4b30db7](4b30db7))
* **theme:** legacy margins ([#2262](#2262)) ([299c869](299c869))

### Features

* increase tooltip width to 500px and truncate items to 2 lines ([#2261](#2261)) ([afdef1c](afdef1c))
@markov00 markov00 mentioned this pull request Dec 20, 2023
Merged
1 task
markov00 added a commit to elastic/kibana that referenced this pull request Dec 21, 2023
@renovate @markov00
Update dependency @elastic/charts to v61.2.0 (main) (#173329)
68883b3 
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [@elastic/charts](https://togithub.com/elastic/elastic-charts) |
[`61.0.3` ->
`61.2.0`](https://renovatebot.com/diffs/npm/@elastic%2fcharts/61.0.3/61.2.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@elastic%2fcharts/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@elastic%2fcharts/61.0.3/61.2.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>elastic/elastic-charts (@&#8203;elastic/charts)</summary>

###
[61.2.0](elastic/elastic-charts@v61.1.0...v61.2.0)
(2023-12-19)


##### Bug Fixes

* **deps:** update dependency @elastic/eui to ^91.1.0
([#2267](elastic/elastic-charts#2267))
([308e974](elastic/elastic-charts@308e974))
* **deps:** update dependency @elastic/eui to ^91.2.0
([#2268](elastic/elastic-charts#2268))
([29cdcb3](elastic/elastic-charts@29cdcb3))
* **metric:** background colors and sparkline rendering
([#2255](elastic/elastic-charts#2255))
([5abddfc](elastic/elastic-charts@5abddfc))
* **partition:** rendering with small radius
([#2273](elastic/elastic-charts#2273))
([95a8537](elastic/elastic-charts@95a8537))
* **partition:** zero value sectors cause max stack call
([#2260](elastic/elastic-charts#2260))
([4b30db7](elastic/elastic-charts@4b30db7))
* **theme:** legacy margins
([#2262](elastic/elastic-charts#2262))
([299c869](elastic/elastic-charts@299c869))


##### Features

* increase tooltip width to 500px and truncate items to 2 lines
([#2261](elastic/elastic-charts#2261))
([afdef1c](elastic/elastic-charts@afdef1c))

###
[`v61.1.0`](https://togithub.com/elastic/elastic-charts/blob/HEAD/CHANGELOG.md#6110-2023-11-20)

[Compare
Source](https://togithub.com/elastic/elastic-charts/compare/v61.0.3...v61.1.0)

##### Bug Fixes

- **deps:** update dependency
[@&#8203;elastic/eui](https://togithub.com/elastic/eui) to v91
([#&#8203;2233](https://togithub.com/elastic/elastic-charts/issues/2233))
([e89f623](https://togithub.com/elastic/elastic-charts/commit/e89f623792312c4f6b609ebb975de0800f3c297e))
- **metric:** add option to set empty cell background color
([#&#8203;2247](https://togithub.com/elastic/elastic-charts/issues/2247))
([6a9fb86](https://togithub.com/elastic/elastic-charts/commit/6a9fb86bee5212a47060c5070f260961097014b4))
- **metric:** background color for bar with interactions
([#&#8203;2248](https://togithub.com/elastic/elastic-charts/issues/2248))
([dcb56fa](https://togithub.com/elastic/elastic-charts/commit/dcb56fa08540631a9b3b0e588352ee6daf3d54a0))

##### Features

- **bullet:** improve header layout and positioning
([#&#8203;2243](https://togithub.com/elastic/elastic-charts/issues/2243))
([b3a95d2](https://togithub.com/elastic/elastic-charts/commit/b3a95d24fb02690ca6599622352c743c04624690))
- **bullet:** new design style and implementation
([#&#8203;2156](https://togithub.com/elastic/elastic-charts/issues/2156))
([fca6cdd](https://togithub.com/elastic/elastic-charts/commit/fca6cdd5bc34a65c0792dbab7d756404bf43501b))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/elastic/kibana).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuMTAzLjEiLCJ0YXJnZXRCcmFuY2giOiJtYWluIn0=-->

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Marco Vettorello <marco.vettorello@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nickofthyme nickofthyme nickofthyme approved these changes

Assignees
No one assigned
Labels
bug Something isn't working :data Data/series/scales related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Pie chart throw if outerRadius is zero or negative
2 participants
@markov00 @nickofthyme