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(partition): linked label on a larger than 180 degree slice #726

Merged
merged 2 commits into from
Jun 30, 2020

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Jun 29, 2020

Summary

Fixes #699
The issue arose when a single large (>180 degrees) slice is forced to have a linked label, eg. via a large or infinite maximumSection value (the default is, fill text)

This:
image

gets fixed:
image

The root cause was that the preexisting calculation simply took the midpoint of the start and end angle, and if it was shorter to go along the other arc of the circle, then that's what it did. So it always resulted in an exact overlap with two slices where one was over 180 degrees and linked labels were forced.

There are two other mock updates. While the previous images were OK - as there is a singleton slice - the new version is more correct. It now picks the midpoint between the starting angle (0deg) and the ending angle (360deg) which is 180deg which is on the bottom.

Checklist

Delete any items that are not applicable to this PR.

  • Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
  • This was checked for cross-browser compatibility, including a check against IE11
  • Proper documentation or storybook story was added for features that require explanation or tutorials
  • Unit tests were updated or added to match the most common scenarios - tested via integration (spec to image) test

@monfera monfera added the :partition Partition/PieChart/Donut/Sunburst/Treemap chart related label Jun 29, 2020
@monfera monfera requested a review from markov00 June 29, 2020 22:09
@monfera monfera self-assigned this Jun 29, 2020
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.

LGTM

Copy link
Member

@markov00 markov00 left a comment

Choose a reason for hiding this comment

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

LGTM tested with the example code from the original issue

@markov00 markov00 merged commit 2504bbe into master Jun 30, 2020
@markov00 markov00 deleted the large-angle-linked-label branch June 30, 2020 08:06
markov00 pushed a commit that referenced this pull request Jun 30, 2020
# [19.7.0](v19.6.3...v19.7.0) (2020-06-30)

### Bug Fixes

* **partition:** linked label on a larger than 180 degree slice ([#726](#726)) ([2504bbe](2504bbe)), closes [#699](#699)

### Features

* add domain padding ([#707](#707)) ([15c78c1](15c78c1)), closes [#706](#706)
@markov00
Copy link
Member

🎉 This PR is included in version 19.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label Jun 30, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:partition Partition/PieChart/Donut/Sunburst/Treemap chart related released Issue released publicly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Linked labels overlaps each other
3 participants