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): don't fill singleton slice if the inner radius is nonzero #643

Merged
merged 2 commits into from
Apr 22, 2020

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Apr 21, 2020

Summary

Fixes the known bad storybook case and closes #637

It reveals text layout suboptimality in the face of Southern multirow placement in thin rings if the text is long and/or has some spaces, not addressed in this PR (preexisting issue will be linked later); temporary workaround against overzelaous line breaks: use of non-breaking space (unicode in JS string) as seen in the story source.

Bullseye chart?

image

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
  • Story tests were updated or added to match the most common scenarios

@monfera monfera self-assigned this Apr 21, 2020
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.

Code LGTM, tested locally

@markov00 markov00 added :partition Partition/PieChart/Donut/Sunburst/Treemap chart related bug Something isn't working labels Apr 22, 2020
@markov00 markov00 changed the title fix: don't fill singleton slice if the inner radius is nonzero fix(partition): don't fill singleton slice if the inner radius is nonzero Apr 22, 2020
@monfera monfera merged this pull request into elastic:master Apr 22, 2020
@monfera monfera deleted the singular-multiring-fix branch April 22, 2020 10:40
markov00 pushed a commit that referenced this pull request Apr 22, 2020
This commit fix the partition chart avoiding filling a single slice
if the inner radius is nonzero.

fix #637
markov00 pushed a commit that referenced this pull request Apr 22, 2020
# [18.4.0](v18.3.0...v18.4.0) (2020-04-22)

### Bug Fixes

* **partition:** single slice wrong text positioning ([#643](#643)) ([6298d36](6298d36)), closes [#637](#637)
* **treemap:** align onElementClick parameters to sunburst ([#636](#636)) ([2c1d224](2c1d224)), closes [#624](#624)

### Features

* allow colorVariant option for series specific color styles ([#630](#630)) ([e5a206d](e5a206d))
* **series:** BubbleSeries (alpha) and markSizeAccessor ([#559](#559)) ([3aa235e](3aa235e))
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this pull request Feb 10, 2022
# [18.4.0](elastic/elastic-charts@v18.3.0...v18.4.0) (2020-04-22)

### Bug Fixes

* **partition:** single slice wrong text positioning ([opensearch-project#643](elastic/elastic-charts#643)) ([f8b5b8a](elastic/elastic-charts@f8b5b8a)), closes [opensearch-project#637](elastic/elastic-charts#637)
* **treemap:** align onElementClick parameters to sunburst ([opensearch-project#636](elastic/elastic-charts#636)) ([8dd87bf](elastic/elastic-charts@8dd87bf)), closes [opensearch-project#624](elastic/elastic-charts#624)

### Features

* allow colorVariant option for series specific color styles ([opensearch-project#630](elastic/elastic-charts#630)) ([e2444ef](elastic/elastic-charts@e2444ef))
* **series:** BubbleSeries (alpha) and markSizeAccessor ([opensearch-project#559](elastic/elastic-charts#559)) ([85d9bda](elastic/elastic-charts@85d9bda))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working :partition Partition/PieChart/Donut/Sunburst/Treemap chart related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Two layered pie chart issue in case of only 1 data point
2 participants