[material_ui] Allow custom MaterialInkController implementations - #12833
[material_ui] Allow custom MaterialInkController implementations#12833Sanaullah49 wants to merge 2 commits into
Conversation
InkFeature cast the controller to the private _RenderInkFeatures type, so any custom MaterialInkController threw a TypeError. Store the public interface, add removeInkFeature, and skip painting when the controller is not a RenderObject. Fixes flutter/flutter#192060
There was a problem hiding this comment.
Code Review
This pull request enables custom MaterialInkController implementations by exposing removeInkFeature on the interface and removing private type casts in InkFeature. Feedback highlights that the current implementation's assertions and early-return checks in _paint will prevent custom delegating controllers from functioning correctly, and suggests relaxing the assertion and walking the render tree to find the correct paint context.
| void addInkFeature(InkFeature feature) { | ||
| assert(!feature._debugDisposed); | ||
| assert(feature._controller == this); | ||
| assert(identical(feature.controller, this)); |
There was a problem hiding this comment.
The assert assert(identical(feature.controller, this)) will fail if a custom delegating controller delegates addInkFeature to the host _RenderInkFeatures. Since _RenderInkFeatures is a private class, any custom controller that actually wants to paint ink features must delegate to the host. To support this, we should relax the assert to allow custom controllers that are not RenderObjects.
assert(feature.controller is! RenderObject || identical(feature.controller, this));There was a problem hiding this comment.
Updated: the assert now allows non-RenderObject controllers that delegate addInkFeature to the host Material.
| if (controller is! RenderObject) { | ||
| return; | ||
| } | ||
| final paintContext = controller as RenderObject; |
There was a problem hiding this comment.
If a custom delegating controller is used, controller is! RenderObject will be true, causing _paint to return early and preventing any ink features from painting. To support custom delegating controllers, we can walk up the render tree from referenceBox to find the actual _RenderInkFeatures ancestor when the controller is not a RenderObject.
RenderObject? paintContext;
if (controller is RenderObject) {
paintContext = controller as RenderObject;
} else {
RenderObject? ancestor = referenceBox.parent;
while (ancestor != null && ancestor is! _RenderInkFeatures) {
ancestor = ancestor.parent;
}
paintContext = ancestor;
}
if (paintContext == null) {
return;
}There was a problem hiding this comment.
Updated: when the controller is not a RenderObject, _paint walks from referenceBox to the enclosing Material ink render object. Added a paint regression test for a delegating controller.
Relax addInkFeature identity checks for non-RenderObject controllers and resolve the paint transform from the enclosing Material when the feature controller only delegates. Adds a paint regression test.
Removes the private
_RenderInkFeaturescast fromInkFeatureso customMaterialInkControllerimplementations no longer throw aTypeError.Also adds
MaterialInkController.removeInkFeature, and paints ink via the enclosing Material ink render object when the controller itself is not aRenderObject(delegating controllers).Fixes flutter/flutter#192060
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2