feat(partition): Flame and icicle chart

[monfera]

Summary

Adds icicle chart as one of the <Partition> layout options next to treemap and sunburst
Also closes #964
And closes #957

Comparison of icicle/flame chart with treemap:

• icicle chart is suitable for at least a couple dozen layers, while treemap is not well readable past 2-3 layers; ideal is 1-2 layers
• icicle chart layout can be stable with the addition/removal of data, while treemap either gets relayouted (may jump) or sticks to original layout but suboptimal aspect ratio of subdivisions
• icicle chart can represent some dimension horizontally, eg. time (like flame chart) or can be alphabetically ordered aggregates (like flame graph) or even, like existing partitioning charts, in descending order of sizes
• lengths are easier to compare than areas

Comparison of icicle/flame chart with sunburst/pie:

• icicle chart is suitable for at least a couple dozen layers, while sunburst/pie is not well readable past 2-3 layers; ideal is 1 layers (pie) to 3 layers
• icicle chart represents values consistently and in a way that's easy to compare, while sunburst magnifies values (projected as sector arc lengths) as radius gets ever larger for less and less aggregated layers (this can be a pro or cons, depending)
• icicle chart has optimal text boxes (straight, horizontal), which also makes it better for future exploratory action eg. drilldown (eg. click on a bar to show only its descendants) - tweening is possible for text too
• icicle chart makes better use of the space, as the container is rectangular
• ordering of slices is more flexible than with pie/sunburst

In short, icicle, or its upright version, the flame chart, are preferable over pie, sunburst and treemap in many general situations as well as in observability eg. monitoring and APM. Due to its prevalence, user familiarity among would-be users is high.

Icicle vs flame: the choice is mostly down to layout on a dashboard; customs for the task or organization, and aesthetic choices.

Also tightens a few types and implementation bits (see the initial commits)

Checklist

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

Todo, in a post @alpha PR:

• Either add flamegraph with this PR or remove references to it
• Image mocks with special cases
• Better text positioning
• (discuss) generalize and expose the FlatTree function, currently in the story, that transforms a nested JS object tree into the flat representation used by <Partition>

[codecov-io]

Codecov Report

Merging #965 (5e9559b) into master (40e6a8a) will increase coverage by 0.57%.
The diff coverage is 80.58%.

@@            Coverage Diff             @@
##           master     #965      +/-   ##
==========================================
+ Coverage   71.01%   71.58%   +0.57%     
==========================================
  Files         344      360      +16     
  Lines       10929    11266     +337     
  Branches     2380     2417      +37     
==========================================
+ Hits         7761     8065     +304     
- Misses       3154     3181      +27     
- Partials       14       20       +6     

Flag	Coverage Δ
unittests	71.08% <80.58%> (+0.07%)	⬆️

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

Impacted Files	Coverage Δ
...types/partition_chart/layout/types/config_types.ts	100.00% <ø> (ø)
...types/partition_chart/renderer/dom/highlighter.tsx	22.64% <28.57%> (+4.89%)	⬆️
...artition_chart/renderer/canvas/canvas_renderers.ts	5.88% <50.00%> (+0.08%)	⬆️
...ypes/partition_chart/layout/viewmodel/viewmodel.ts	83.22% <76.78%> (-6.09%)	⬇️
...hart_types/partition_chart/layout/utils/treemap.ts	98.50% <90.90%> (+0.09%)	⬆️
...es/partition_chart/layout/utils/group_by_rollup.ts	87.09% <100.00%> (ø)
...art_types/partition_chart/layout/utils/sunburst.ts	100.00% <100.00%> (ø)
...tion_chart/layout/viewmodel/hierarchy_of_arrays.ts	100.00% <100.00%> (ø)
...hart_types/partition_chart/state/selectors/tree.ts	95.00% <100.00%> (+2.14%)	⬆️
src/mocks/series/series.ts	90.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 40e6a8a...5e9559b. Read the comment docs.

[nickofthyme]

Looks great, nice work! I just have a few comments but nothing blocking.

It seems that the render time is a little longer than the current partition charts, do you notice this too?

[nickofthyme]

Do you need these comments still?

[monfera]

I've been somewhat relaxed about leaving property uses present but commented out in mocks, for quick manual tests, eg. during making the non-alpha version; let's chat if such lines should be cleared up in the numerous mocks where they may be present, it could be a follow-up PR with a bunch of other file changes

[markov00]

One reason to remove those comments is that those types can change in the future, these commented out text could be outdated without noticing that

[monfera]

I made the change; getting out of sync wouldn't bother me, or lib developers, but indeed it'd be confusing for someone who only looks at the source sample in storybook (ie. Kibana developer, by and large)

[monfera]

a57b697

[nickofthyme]

Yeah I see your point @monfera but that could be a slippery slope where we just have comments all over the place with some different than others.

I think we could achieve this same effect by creating some mock or story defaults file that came house a specific use case to be used anywhere. Then we can describe the use more explicitly.

[monfera]

@nickofthyme thanks for the review. I noticed the comparative slowness. The tree is 18 levels deep and has quite a few nodes (somewhat wide too); and the legends becomes rather long too (lots of HTML elements). Planning to look into it in the alpha stage

[markov00]

Everything looks good, I suggest to rework the legend items sorting before waiting for the sorting PR I'm working on for two reasons:

• the legend sorting I'm working is implemented only for xy charts
• the legend items structure depends mostly on the data model that varies from chart type to chart type

[markov00]

This is just a clean up on the PR, but seriously looking at that getRootArrayNode function, it doesn't return strictly an ArrayNode, the STATISTICS_KEY and SORT_INDEX_KEY are not available there.
It's possible that for the root note these values are ignored in the current code, but that doesn't mean that we can easily remember that fact when refactoring/reworking on that file. It can leads to bug if we believe that also the root node is of type ArrayNode
Fixing the type here can go in two direction:

• add missing keys in the bootstrap object with some default values, declare the bootstrap as Exclude<ArrayNode, parent
• relax the ArrayNode type for the missing keys
• create a RootArrayNode type without these missing keys

This task doesn't need to be handled in this PR, but it's just an example of why ts ignoring or putting as can deal to issues.

[monfera]

Thanks, good points!

[monfera]

Just pondering... the possible issue with points 1 and 3 is that it'll no longer be possible to model the entire tree as having homogeneous type; I'm sure it's solvable with type guards and such, with some more work.

With option 2, it'd relax the type for all uses, and I wouldn't want to water it up.

I think adding the missing properties to this root node would be a compact, local solution, but it'd still need the as assertion, because the [PARENT_KEY] prop demands an ArrayNode.

So my current plan is to see what I can do locally (eg. just adding the missing props but leaving in place the as) and continue exploring and likely, discussing, outside this PR

[monfera]

OK the current solution is, use Omit to exclude PARENT_KEY in the assignment:

function getRootArrayNode(): ArrayNode {
  const children: HierarchyOfArrays = [];
  const bootstrap: Omit<ArrayNode, typeof PARENT_KEY> = {
    [AGGREGATE_KEY]: NaN,
    [DEPTH_KEY]: NaN,
    [CHILDREN_KEY]: children,
    [INPUT_KEY]: [] as number[],
    [PATH_KEY]: [] as number[],
    [SORT_INDEX_KEY]: 0,
    [STATISTICS_KEY]: { globalAggregate: 0 },
  };
  return { ...bootstrap, [PARENT_KEY]: bootstrap } as ArrayNode; // TS doesn't yet handle bootstrapping but the `Omit` above retains guarantee for all props except `[PARENT_KEY`
}

This way, there's still a need for as, because TS can't type-bootstrap a self-referential object, but it's way better, because should anything in ArrayNode change, the bootstrap object must reflect that. So the only part where the type checker is asked to trust the human is the presence of PARENT_KEY which is a local, single-line assignment right there

[monfera]

f9e51f9

[markov00]

One reason to remove those comments is that those types can change in the future, these commented out text could be outdated without noticing that

[markov00]

🎉 This PR is included in version 24.5.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀