-
Notifications
You must be signed in to change notification settings - Fork 22
feat: add onScaleEnd, vertexSolidSetter and edgeSolidSetter to options
#33
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
base: main
Are you sure you want to change the base?
Conversation
| late String tag; | ||
|
|
||
| bool solid = true; // the color | ||
| bool solid = false; // the color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NoachDev
I changed the default value to false to maintain the original features. ( It's the same in Edge. )
If there are no issues, I will proceed with the merger.
You can refer to persistence_demo
..vertexSolidSetter = ((vertex, paint) {
paint.color = vertex.colors.first;
return paint;
})
..edgeSolidSetter = ((edge, paint) {
paint.color = edge.start.colors.lastOrNull ?? Colors.white;
return paint;
})There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohhh, your version with setters is much better than my initial idea!
I think the UI props could benefit from better encapsulation, similar to Flutter's container decorations, while still maintaining the granular definition of vertex-by-vertex. I have some new features to propose in a pull request. Could this be a potential suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely possible, as it aligns more closely with developers’ established habits. Although I haven’t delved deeply into container decoration, I sense that the mechanism for periodic redrawing of the canvas differs from setState. I wonder if this difference aligns with your perspective.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mentioned the decoration of container because it is a good visualization of a complete separation of design from other resources, and if implemented, it will influence "globally" - all vertices, all edges. But the idea of introducing the Decoration themselves might be possible, similar features developed in /shapes and /options already existing in this class. However, this will be a hard work of refactoring, probably unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does offer greater control over vertex styles.
If you’re interested, go ahead and experiment. The control over vertexes in version 1.x utilized Flame’s ShapeComponent. However, Flame is no longer available. Nevertheless, you might find the concept useful.
Ultimately, the decision is yours. I only mentioned this in passing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, thanks for the better clarification
In onScaleEnd, fix the issue where the onVertexTapUp callback cannot be triggered again when a vertex was dragged.
Dev habit change