You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 an Observer 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 in shouldRepaint(). This works fine, but it requires a lot of boilerplate, so I've been working on an alternative way to handle this.
classMyPainterextendsCustomPainterObserver {
MyMobxObject someObj;
MyPainter(this.someObj);
@overridevoidobservablePaint(Canvas canvas, Size size) {
// Paint here. Observables are watched and updates to the watched// observables will trigger a repaint.
}
@overrideboolshouldRepaint(covariantMyPainter oldDelegate) {
return oldDelegate.someObj != someObj ||super.shouldRepaint(oldDelegate);
}
}
// ...// In the widget tree...CustomPaintObserver(
painter:MyPainter(
// Can pass MobX objects here and everything works as expected.
),
);
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:
The CustomPaintObserver widget must be used in the tree.
The CustomPainterObserver base class must be used.
If shouldRepaint() is overridden, it must or (||) its result with super.shouldRepaint().
observablePaint() must be overridden instead of paint().
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:
My CustomPaintObserver itself renders a CustomPaint. This means that all constructor arguments need to be forwarded, and any future changes to CustomPaint need to be mirrored in CustomPaintObserver. It would be nice to avoid this maintenance overhead.
In order to trigger a repaint the 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:
SecondFlight
changed the title
Observable CustomPainter
Observable CustomPainter (willing to contribute code)
Mar 24, 2023
Just my two cents (I am a maintainer of the repo though not the main one; btw I know you from flutter_rust_bridge ;) )
use delegate instead of inheritance: your CustomPainterObserver do not need to extend CustomPainter. Then there is no problem of "observablePaint", "|| super.shouldRepaint()" etc.
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: