react-vis icon indicating copy to clipboard operation
react-vis copied to clipboard

feat(treemap): Extend tree map API.

Open Akiyamka opened this issue 6 years ago • 9 comments

Motivation: For creating complex tree map example we need more control over rendering. Adding these props allows you to create, for example, nested treemaps. Example

Akiyamka avatar Jan 18 '19 08:01 Akiyamka

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jan 18 '19 08:01 CLAassistant

Hey @Akiyamka, This is great work so far! Mind adding some tests and an example to the showcase to document (in code) how these feature work?

mcnuttandrew avatar Jan 18 '19 16:01 mcnuttandrew

@mcnuttandrew done, I added example in showcase and few more options for visual styling.

Akiyamka avatar Jan 24 '19 08:01 Akiyamka

@mcnuttandrew, please check changes, is everything OK or is there anything else to change?

Akiyamka avatar Jan 30 '19 09:01 Akiyamka

Hey @Akiyamka would you mind taking a look at both of my comments from the previous review (remove unused file and add tests)

mcnuttandrew avatar Jan 30 '19 21:01 mcnuttandrew

I tried downloading your branch to play around with your example and there are few bugs

  • Clicking on leaf nodes causes an error which causes the whole component to crash
  • It appears to crash on SVG mode

Also a few comments

  • I think it is not a good approach to include dangerouslySetInnerHTML, instead add a class to that component (zoomable-treemap or something) and add those styles examples.scss targeting only that component.
  • i turned on animation, it's pretty fun for your example

mcnuttandrew avatar Jan 30 '19 21:01 mcnuttandrew

@mcnuttandrew, thanks for the review! I fixed crash in svg mode, but unfortunately I can't make it work the same way as in dom mode, because the component has different APIs in these modes. For example, clicking on a sheet does not return the sheet in svg mode. I think I need another pull request to fix this.

Akiyamka avatar Jan 31 '19 09:01 Akiyamka

is this going to be merged soon? i'd love to use it :)

id0Sch avatar Jan 07 '20 15:01 id0Sch

I tried @Akiyamka's branch and it seems to work flawlessly. What is impeding this from being merged?

cleopatra17 avatar Jun 01 '21 11:06 cleopatra17