feat: add debug state for partition charts #1117

markov00 · 2021-04-13T08:52:30Z

Summary

Add the debug state for partition charts with the following type signature:

type PartitionDebugState = {
  panelTitle: string; // the title of the small multiple, empty if not small multiple
  partitions: Array<{
    name: string; // the name of the slice, before applying the formatter (the string is the same as it was in the data array)
    depth: number; // the layer of the slice when using multilayered pies/treemap
    color: string; // usually in rgba, better to use a color utility function in FTR to convert this into what you really need. The format can change in the future, so better be decoupled from that
    value: number; // the actual value of the slice
    coords: [Pixels,Pixels]; // a clickable central coordinate of the slice/sector
  }>;
};

fix #917

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)
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

stratoula · 2021-04-13T12:28:16Z

@markov00 I think we are fine, I get all the data I need for all my test cases 🎉

markov00 · 2021-04-13T13:32:27Z

jenkins test this please

nickofthyme

Nice work, seems appropriate to make a new partition property in the DebugState as there is not much crossover.

nickofthyme · 2021-04-13T16:16:06Z

src/components/chart_status.tsx


  return {
    rendered: state.chartRendered,
    renderedCount: state.chartRenderedCount,
-    onRenderChange: settings.onRenderChange,
-    debugState: settings.debugState ? getDebugStateSelector(state) : null,
+    onRenderChange,


is this method still bound correctly if needed?

I hope so, haven't checked

it works fine

src/chart_types/partition_chart/state/selectors/get_debug_state.test.ts

monfera · 2021-04-15T08:47:49Z

src/chart_types/partition_chart/state/selectors/get_debug_state.ts

+}
+
+function getCoordsForRectangle({ x0, x1, y1px, y0px }: QuadViewModel, diskCenter: PointObject): [Pixels, Pixels] {
+  const y = y0px + (y1px - y0px) / 2 + diskCenter.y;


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

monfera

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

monfera · 2021-04-15T08:50:49Z

src/chart_types/partition_chart/state/selectors/get_debug_state.ts

+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];


If we want to put short coords in the data-ech-debug-state, it could be rounded here

done 211e439

codecov-io · 2021-04-15T10:25:54Z

Codecov Report

Merging #1117 (b29d541) into master (c1b59f2) will increase coverage by 0.63%.
The diff coverage is 96.96%.

@@            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

Flag	Coverage Δ
unittests	`72.47% <96.96%> (+0.20%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../chart_types/partition_chart/state/chart_state.tsx	`70.68% <66.66%> (+0.51%)`	⬆️
...partition_chart/state/selectors/get_debug_state.ts	`100.00% <100.00%> (ø)`
src/components/chart_status.tsx	`91.66% <100.00%> (ø)`
src/mocks/store/index.ts	`100.00% <0.00%> (ø)`
src/mocks/geometries.ts	`92.85% <0.00%> (ø)`
src/mocks/index.ts	`100.00% <0.00%> (ø)`
src/mocks/store/store.ts	`92.59% <0.00%> (ø)`
src/mocks/scale/scale.ts	`77.77% <0.00%> (ø)`
src/mocks/theme.ts	`86.66% <0.00%> (ø)`
src/mocks/series/data.ts	`100.00% <0.00%> (ø)`
... and 15 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1b59f2...b29d541. Read the comment docs.

# [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)

nickofthyme · 2021-04-15T10:38:59Z

🎉 This PR is included in version 28.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

markov00 added 2 commits April 13, 2021 10:48

feat: add debug state for partition charts

e473e8a

docs: update APIs

6d1fe71

markov00 requested a review from monfera April 13, 2021 08:52

markov00 added 2 commits April 13, 2021 12:13

feat: compute sector/rectangle center coords

c63e727

test: add partition click tests

03df3b4

markov00 force-pushed the 2021_04_07-add_debug_state_partition branch from a9a5881 to 03df3b4 Compare April 13, 2021 11:43

markov00 added :partition Partition/PieChart/Donut/Sunburst/Treemap chart related enhancement New feature or request labels Apr 13, 2021

markov00 added 2 commits April 13, 2021 13:43

Merge branch 'master' into 2021_04_07-add_debug_state_partition

7e4a8fe

test: add mosaic

7dda08c

markov00 marked this pull request as ready for review April 13, 2021 11:48

markov00 requested review from rshen91, nickofthyme and stratoula April 13, 2021 11:48

fix: use debugState and not debug prop

06e3716

nickofthyme approved these changes Apr 13, 2021

View reviewed changes

monfera reviewed Apr 15, 2021

View reviewed changes

monfera approved these changes Apr 15, 2021

View reviewed changes

monfera reviewed Apr 15, 2021

View reviewed changes

markov00 added 2 commits April 15, 2021 11:48

refactor: add round to pixel, fix stories

211e439

Merge branch 'master' into 2021_04_07-add_debug_state_partition

b29d541

markov00 enabled auto-merge (squash) April 15, 2021 10:12

markov00 disabled auto-merge April 15, 2021 10:14

markov00 merged commit d7fc206 into elastic:master Apr 15, 2021

markov00 deleted the 2021_04_07-add_debug_state_partition branch April 15, 2021 10:28

nickofthyme added the released Issue released publicly label Apr 15, 2021

nickofthyme mentioned this pull request Apr 15, 2021

Update dependency @elastic/charts to v28.2.0 (master) elastic/kibana#97005

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add debug state for partition charts #1117

feat: add debug state for partition charts #1117

markov00 commented Apr 13, 2021 •

edited

Loading

stratoula commented Apr 13, 2021

markov00 commented Apr 13, 2021

nickofthyme left a comment

nickofthyme Apr 13, 2021

markov00 Apr 13, 2021

markov00 Apr 15, 2021

monfera Apr 15, 2021

monfera left a comment •

edited

Loading

monfera Apr 15, 2021

markov00 Apr 15, 2021

codecov-io commented Apr 15, 2021

nickofthyme commented Apr 15, 2021

feat: add debug state for partition charts #1117

feat: add debug state for partition charts #1117

Conversation

markov00 commented Apr 13, 2021 • edited Loading

Summary

Checklist

stratoula commented Apr 13, 2021

markov00 commented Apr 13, 2021

nickofthyme left a comment

Choose a reason for hiding this comment

nickofthyme Apr 13, 2021

Choose a reason for hiding this comment

markov00 Apr 13, 2021

Choose a reason for hiding this comment

markov00 Apr 15, 2021

Choose a reason for hiding this comment

monfera Apr 15, 2021

Choose a reason for hiding this comment

monfera left a comment • edited Loading

Choose a reason for hiding this comment

monfera Apr 15, 2021

Choose a reason for hiding this comment

markov00 Apr 15, 2021

Choose a reason for hiding this comment

codecov-io commented Apr 15, 2021

Codecov Report

nickofthyme commented Apr 15, 2021

markov00 commented Apr 13, 2021 •

edited

Loading

monfera left a comment •

edited

Loading