Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom node, record types #350

Closed
drfbwilliams opened this issue Jun 18, 2021 · 3 comments
Closed

Custom node, record types #350

drfbwilliams opened this issue Jun 18, 2021 · 3 comments
Labels

Comments

@drfbwilliams
Copy link

Hi,

I had previously implemented the lib and thought I would go ahead and attempt to update to 2.02 - the wheels fell of that pretty quickly.

In particular, I note that you've done an amazing job rewriting everything as TS first - yet all the examples are still JS etc - clearly it's a time consuming task to try and get it all done, so please don't read this as criticism.

The issues I am having is around your choice to implement the new node structure as:

interface RawNodeDatum { name: string; attributes?: Record<string, string>; children?: RawNodeDatum[]; }

This could absolutely be just my lack of understanding here, but previously my node structure was this:

`export type dataNodeAttributeType = {
department: string;
drfType: string;
isSelected: boolean;
elementType: string;
showElementState: boolean;
isRootMode: boolean;
};

export type dataNodeType = {
name: string;
id: string;
attributes?: dataNodeAttributeType;
children?: dataNodeType[];
};
`

It would be a lot of work to now have to push the type into a JSON object, serialise and deserialise, just to get my node values squeezed through the record type. The id you see in my dataNodeType is particularly important as well.

Given that you can see my required node structure, can you please provide a quick bit of code to explain how I could now use the same structure in V2.

Many thanks,

@drfbwilliams
Copy link
Author

After a day, I guess I have my own answer in that the Record<string, string> does indeed mean that the only way to pass custom data is by storing a JSON string in the record.

And as I assumed - yes it was a lot of work to make this change happen. I am not clear as to what the benefits are, though I am sure you have reasons for it.

Thanks for all the hard work put into your project - it is much appreciated.

bkrem added a commit that referenced this issue Jun 23, 2021
`attributes` was made unintentionally restrictive during the v2 refactor to
Typescript, by moving from a loose implicit `object` type to `Record<string,
string>`.
This commit widens the used `Record` type to additionally accept `number`
and `boolean` primitives as possible values.
@bkrem bkrem added the bug label Jun 23, 2021
@bkrem
Copy link
Owner

bkrem commented Jun 23, 2021

Hi @drfbwilliams,

I've had some time to work on the library the last couple days and catch up on issues. Sorry to hear you were having trouble with the move to v2.
There are so many possible use cases in play (which also makes it great to see what people build of course) that I didn't realise that the typings for attributes had become too restrictive when compared to the loose implicit behaviour in v1.

I've widened the RawNodeDatum interface to additionally accept boolean and number primitives as values in v2.0.4. Hope that helps (re-)simplify the implementation on your end 👍

I'd also recommend checking out the docs on renderCustomNodeElement if you haven't yet. It was added in v2, specifically to enable much greater customisation of nodes and to act as a de facto "escape hatch" for when the SVG namespace becomes too restrictive for the wanted use case.

@drfbwilliams
Copy link
Author

@bkrem thanks for the response - and please, no need to apologise for anything, like so many of these projects, you are putting in a lot work to benefit a lot of people (like myself), often without the thanks that should be given.

With respect to renderCustomNodeElement - yes I did take a look - thanks for the examples on codesandbox. My nodes are relatively complex elements with selected states and animated expand/collapse states etc. I don't have any plans to make them even more complex, but you never know. I will shout out if something beats me up.

Thanks again.

Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants