dgreene1 / react-accessible-treeview

A react component that implements the treeview pattern as described by the WAI-ARIA Authoring Practices.
https://dgreene1.github.io/react-accessible-treeview
MIT License
277 stars 37 forks source link

Propagate additional information to nodes with INode metadata #117

Closed megbailey closed 1 year ago

megbailey commented 1 year ago

Attempting to resolve https://github.com/dgreene1/react-accessible-treeview/issues/86 and the comments brought up in https://github.com/dgreene1/react-accessible-treeview/pull/101

dgreene1 commented 1 year ago

@mellis481 this PR has the fixes i requested. It was very nice of @megbailey to make the changes that the previous submitter was asked to make. Can you and @yhy-1 get this approved and merged?

Sincerely, from the beach, Dan

dgreene1 commented 1 year ago

@kpustakhod I fixed the no-any lint issues. @megbailey that makes this solution fully generic and it now even infers the type of the metadata.

Can you all take a look and see if it looks good to you?

dgreene1 commented 1 year ago

@kpustakhod please merge this

sandro768 commented 1 year ago

Please merge it 🥶

dgreene1 commented 1 year ago

Thank you @megbailey for you contributions. Thank you @sandro768 for the push. We love our community. :)

Pearce-Ropion commented 1 year ago

What is the purpose of doing a shallow clone on the metadata object? Presumably, metadata is just used for additional data that can be used to render large trees and will likely only contain static properties. If anything, I would assume (as a user) that any metadata object that is passed to a given node could be mutated as that is how javascript objects typically work. By shallow cloning the metadata object, we are effectively breaking the expected behavior of the data being passed to the tree.

Furthermore, I don't think nodes should only accept a non-nested metadata object purely so that the flattenTree function can do a shallow clone. For cases where the user is manually building the INode array, users shouldn't be limited to a single level of metadata since we aren't doing any additional metadata manipulation for them.

And finally, boolean should be added to the IFlatMetadata type.

edit: I wrote up a longer post in #129