feat: add basic functionality for icicle chart to display static data #165
Conversation
adds functionality for the icicle chart to have data passed in and display (static, no interactions yet)
Codecov Report
@@ Coverage Diff @@
## master #165 +/- ##
==========================================
+ Coverage 25.9% 28.65% +2.74%
==========================================
Files 8 9 +1
Lines 166 178 +12
Branches 10 14 +4
==========================================
+ Hits 43 51 +8
- Misses 122 123 +1
- Partials 1 4 +3
Continue to review full report at Codecov.
|
|
do you have any screenshots? |
| .selectAll('*') | ||
| .remove(); | ||
|
|
||
| const svg = d3Select(this.chartRef.current) |
There was a problem hiding this comment.
I'm curious why you aren't using a library like @vx that plays better with react and renders visualizations/utilizes d3 more declaratively.
There was a problem hiding this comment.
I have no good answer for that other than being not very familiar with vx... I recall seeing it in the event-flow viz if I remember correctly but didn't look too much into it. Will definitely look more into that library tomorrow in the morning!
There was a problem hiding this comment.
Very good point! The visualization will have some animation as well and it doesn't look like vx supports it. Although we could use other react animation libraries with vx. I think given Chris's deadlines, I suggest that this is something we look into later on. I am not a big fan of how unreadable d3 either but fortunately, I don't think there is much more than what is already in this PR.
|
Yes, I have some some screenshots! This is what it looks like currently in Storybook with the intention that it is not the final look. Since this PR was already getting a little big, I wanted to make it in a separate one:
Additionally, the README for this package has a screenshot of a prototype that is slightly outdated (we're still going through design, but have made some changes to spacing between icicles, etc.) |
| @@ -29,14 +31,41 @@ interface Props { | |||
| y: number; | |||
| }; | |||
| color: (name: string) => string; | |||
| contentRenderer: () => void; | |||
| contentRenderer: (...args: any[]) => void; | |||
There was a problem hiding this comment.
Can we more more explicit on the type?
There was a problem hiding this comment.
Updated to include the types, BaseType is documented here:
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/d3-selection/index.d.ts#L21
…t#165) Updates the requirements on [jest-mock-console](https://github.com/bpedersen/jest-mock-console) to permit the latest version. - [Release notes](https://github.com/bpedersen/jest-mock-console/releases) - [Commits](https://github.com/bpedersen/jest-mock-console/commits) Signed-off-by: dependabot[bot] <[email protected]>
…apache-superset#165) * feat: add basic functionality for icicle chart to display static data adds functionality for the icicle chart to have data passed in and display (static, no interactions yet) * feat: increase code coverage to pass check * feat: clarify contentRenderer argument types
🏆 Enhancements
adds functionality for the icicle chart to have data passed in and display (static, no interactions
yet)