-
-
Notifications
You must be signed in to change notification settings - Fork 308
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
Observable CustomPainter
(willing to contribute code)
#908
Comments
SecondFlight
changed the title
Observable
Observable Mar 24, 2023
CustomPainter
CustomPainter
(willing to contribute code)
Just my two cents (I am a maintainer of the repo though not the main one; btw I know you from flutter_rust_bridge ;) )
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi! Thank you for this library. It has helped me increase my productivity and greatly simplify my state and logic code, and it has quickly become my favorite state management solution.
I'm writing an app with a lot of
CustomPaint
widgets that need to draw things based on MobX model objects. Initially, my strategy was to just unwrap the model items inside anObserver
builder, like so:This works okay, but it doesn't scale. For instance, if
modelItem
has a list of child model items, then my strategy was to create an immutable proxy object for those child items and pass a list of those, which allowed me to handle changes inshouldRepaint()
. This works fine, but it requires a lot of boilerplate, so I've been working on an alternative way to handle this.I would be interested in contributing code to provide something like a
CustomPainterObserver
, but I may need some guidance. For my own project I've coded something that accomplishes this: https://github.com/SecondFlight/anthem/blob/fe2d8346d9b046dddcffd0d9c989fd0dd2b27ca4/lib/widgets/basic/mobx_custom_painter.dartIt can be used like so:
I'd be happy to submit this as-is, but there's a couple issues with it. First, it doesn't take semantics into account. This is pretty fixable, but the second issue is the bigger one in my view: it just feels clunky to me. This is especially true when comparing this implementation with the observer widget base classes, which are literally drop-in replacements, i.e. change the base class and you're good to go.
In contrast, my implementation requires the following:
CustomPaintObserver
widget must be used in the tree.CustomPainterObserver
base class must be used.shouldRepaint()
is overridden, it must or (||
) its result withsuper.shouldRepaint()
.observablePaint()
must be overridden instead ofpaint()
.I'd expect either 1 or 2 to be required but optimally not both, and it feels like I should be able to avoid the other requirements as well. I tried to address these in my initial implementation but kept running into roadblocks, and the implementation above was the first that worked for me.
In addition, there are some things I don't like about how my implementation works internally:
CustomPaintObserver
itself renders aCustomPaint
. This means that all constructor arguments need to be forwarded, and any future changes toCustomPaint
need to be mirrored inCustomPaintObserver
. It would be nice to avoid this maintenance overhead.ReactionImpl
triggers a rebuild, which causes the painter to re-evaluate. It feels like this should be more straightforward, something like finding the object in the render tree and marking it as dirty?Please let me know any thoughts or alternative implementation suggestions. I'm happy to contribute, but I want to make sure I'm contributing high-quality code.
The text was updated successfully, but these errors were encountered: