feat: add debug state for partition charts#1117
Conversation
a9a5881 to
03df3b4
Compare
|
@markov00 I think we are fine, I get all the data I need for all my test cases 🎉 |
|
jenkins test this please |
nickofthyme
left a comment
There was a problem hiding this comment.
Nice work, seems appropriate to make a new partition property in the DebugState as there is not much crossover.
| renderedCount: state.chartRenderedCount, | ||
| onRenderChange: settings.onRenderChange, | ||
| debugState: settings.debugState ? getDebugStateSelector(state) : null, | ||
| onRenderChange, |
There was a problem hiding this comment.
is this method still bound correctly if needed?
There was a problem hiding this comment.
I hope so, haven't checked
| } | ||
|
|
||
| function getCoordsForRectangle({ x0, x1, y1px, y0px }: QuadViewModel, diskCenter: PointObject): [Pixels, Pixels] { | ||
| const y = y0px + (y1px - y0px) / 2 + diskCenter.y; |
There was a problem hiding this comment.
Multilayer treemaps and mosaic have overlapping geoms for potential border/coloring around the larger group perimeter, so the mid Y activates a leaf node. It may be fine as the path still includes the non-leaf layers. If it's not desirable, an alternative would be, instead of computing the vertical center of the box, the y could be the top of the box plus a couple of pixels
There was a problem hiding this comment.
Looks fantastic, thank you for having made this 🎉
Worked great with debugState, probably worth committing it i place of or in addition to debug.
Due to special cases, I tested coordinates with:
- pie chart with big margins that push aside the chart
- regular sunburst
- sunburst with single 360 degree concentric disk / rings
- no slices at all
- sunburst small multiples
- two-layer treemap
- mosaic
| function getCoordsForRectangle({ x0, x1, y1px, y0px }: QuadViewModel, diskCenter: PointObject): [Pixels, Pixels] { | ||
| const y = y0px + (y1px - y0px) / 2 + diskCenter.y; | ||
| const x = x0 + (x1 - x0) / 2 + diskCenter.x; | ||
| return [x, y]; |
There was a problem hiding this comment.
If we want to put short coords in the data-ech-debug-state, it could be rounded here
Codecov Report
@@ Coverage Diff @@
## master #1117 +/- ##
==========================================
+ Coverage 72.26% 72.90% +0.63%
==========================================
Files 387 405 +18
Lines 12021 12384 +363
Branches 2629 2679 +50
==========================================
+ Hits 8687 9028 +341
- Misses 3309 3322 +13
- Partials 25 34 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
# [28.2.0](v28.1.0...v28.2.0) (2021-04-15) ### Bug Fixes * **xy:** consider `useDefaultGroupDomain` on scale config ([#1119](#1119)) ([c1b59f2](c1b59f2)), closes [#1087](#1087) ### Features * **a11y:** allow user to pass custom description for screen readers ([#1111](#1111)) ([2ee1b91](2ee1b91)), closes [#1097](#1097) * **partition:** add debuggable state ([#1117](#1117)) ([d7fc206](d7fc206)), closes [#917](#917)
|
🎉 This PR is included in version 28.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
# [28.2.0](elastic/elastic-charts@v28.1.0...v28.2.0) (2021-04-15) ### Bug Fixes * **xy:** consider `useDefaultGroupDomain` on scale config ([opensearch-project#1119](elastic/elastic-charts#1119)) ([269ff1a](elastic/elastic-charts@269ff1a)), closes [opensearch-project#1087](elastic/elastic-charts#1087) ### Features * **a11y:** allow user to pass custom description for screen readers ([opensearch-project#1111](elastic/elastic-charts#1111)) ([a0020ba](elastic/elastic-charts@a0020ba)), closes [#1097](elastic/elastic-charts#1097) * **partition:** add debuggable state ([opensearch-project#1117](elastic/elastic-charts#1117)) ([08f8baf](elastic/elastic-charts@08f8baf)), closes [opensearch-project#917](elastic/elastic-charts#917)
Summary
Add the debug state for partition charts with the following type signature:
fix #917
Checklist
Delete any items that are not applicable to this PR.
src/index.ts(and stories only import from../srcexcept for test data & storybook)