Comments (8)
Hi @Avivhdr.
Nice catch. There are a few points in here.
onChange
is actually required for real, if it isn't supplied the expansion will throw an error, thatonChange
should be the one injected by your child function (https://github.com/diogofcunha/react-virtualized-tree//blob/master/demo/src/examples/LargeCollection.js#L33) - I'm waiting on react 17 to implement this with the new context API and kill off the need to inject this properties that are the real definition of context, it just looked quite messy with the old context apionMeasure
should be optional.- on the
nodeShape
I'm tempted to say that we should change it to be eithernodeShape
orobject
to make it as extensible as possible.
What do you think? Would you be able to open a PR addressing points 2 and 3? I would be eternally grateful 😺, otherwise I will probably do it over weekend.
Cheers.
from react-virtualized-tree.
Thanks for the fast reply @diogofcunha.
how would you recommend to do this change (No. 3) ?
Should the isRequired be removed from the name prop or should the Tree component receives another prop called nodeShape that will determine how the shape of the node should be, like this:
const nodeShape = {
groups: PropTypes.arrayOf(PropTypes.shape({
id: PropTypes.number,
name: PropTypes.string,
state: PropTypes.shape({
expendable: PropTypes.bool,
}),
children: PropTypes.arrayOf(PropTypes.shape({
color: PropTypes,
id: PropTypes.number,
name: PropTypes.string,
licensePlate: PropTypes.string,
// {...more children props}
})),
// {...more group props}
}),
}
......
<Tree nodes={nodes} nodeMarginLeft={0}> nodeShape={nodeShape}
{
({ node, ...restProps }) =>
<Expandable node={node} onChange={this.handleChange}>
{.......}
</Expandable>
}
</Tree>
In short, instruct me and i'll do my best!
from react-virtualized-tree.
Hi @Avivhdr.
I don't think it's necessary to inject the nodeShape as a prop, specially because I will probably rewrite the library in typescript in the near future and it would be ignored anyway.
I was investigating a little bit an what's clear in my mind is that we have a required structure for a node, each is:
id: string | number;
state?: { };
children?: Node[]
It would be good to check that these are valid on runtime to avoid bugs, all the rest is not very relevant to the typechecking on the library side, at the user side people can do stricter typechecks without any problem.
The trick here is that PropTypes are just functions, so if we can make a checker that checks for the required properties and that doesn't throw warnings for extra props it would be amazing.
Of course that has the library also ships with ts definitions that also need to be changed.
I may need to get my hands in the code to figure this out a little bit better
from react-virtualized-tree.
On the other hand I wonder if not having any of these required properties shouldn't just throw an actual descriptive error as it compromises the good behaviour of an application and can prevent a lot of unexpected bugs.
If we just throw (and I know this would be a bold decision) we could in theory just say that node is a PropTypes.object
, but let's try to make a proptype checker first
from react-virtualized-tree.
If we define Node as being a function for proptype checking proposes.
The way to define a custom validator is: (source)
function(props, propName, componentName) {
if (!/matchme/.test(props[propName])) {
return new Error(
'Invalid prop `' + propName + '` supplied to' +
' `' + componentName + '`. Validation failed.'
);
}
},
Do you think you can play around with it a little bit so that:
- An error is thrown when id is not supplied
- Works for children too
- Doesn't throw warnings for extra props
I can figure out the typescript part over the weekend
from react-virtualized-tree.
Hi @diogofcunha I'll be happy to try and contribute but first i need to have a clear view on what to do.
I don't undarstand the benefits of creating a custom validator function that will allow extra props and will throw when id is not passed when this can be achieved easily by removing the isRequired from the name prop here: https://github.com/diogofcunha/react-virtualized-tree/blob/master/src/shapes/nodeShapes.js#L11
from react-virtualized-tree.
hey @Avivhdr sorry about the delay I was on holiday.
Let's do that and I will try to figure out a way to have static typing for typescript
from react-virtualized-tree.
from react-virtualized-tree.
Related Issues (20)
- Hiii..Can you please help me in adding folder icon in front of each node ?
- Overlapping text
- Hi..I was not able to retrieve data from Autodesk in real time using R visualizations in Power BI.com.
- Website can not access
- Support Adding new nodes HOT 2
- Example of how to use indexSearch? HOT 2
- Expanding filtered node paths
- Is it possible to import individual components into a react project
- FilteringContainer: Is it possible to add a prop for minimum characters in search?
- props data warning
- Can we add a function to show the attributes of node?
- how to use it with my component
- Can the tree draw/show connecting lines between parent and children to show they are linked
- test
- Can the tree draw/show connecting lines between parent and children to show they are linked
- Add TypeScript support HOT 1
- Improve React Developer Tools support HOT 2
- Menu not appearing in the correct place
- React 18 support
- How to use debouncer? HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from react-virtualized-tree.