Skip to content

[material_ui] Allow custom MaterialInkController implementations - #12833

Open
Sanaullah49 wants to merge 2 commits into
flutter:mainfrom
Sanaullah49:material-ui-inkfeature-controller-api
Open

[material_ui] Allow custom MaterialInkController implementations#12833
Sanaullah49 wants to merge 2 commits into
flutter:mainfrom
Sanaullah49:material-ui-inkfeature-controller-api

Conversation

@Sanaullah49

@Sanaullah49 Sanaullah49 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Removes the private _RenderInkFeatures cast from InkFeature so custom MaterialInkController implementations no longer throw a TypeError.

Also adds MaterialInkController.removeInkFeature, and paints ink via the enclosing Material ink render object when the controller itself is not a RenderObject (delegating controllers).

Fixes flutter/flutter#192060

Pre-Review Checklist

Footnotes

  1. 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

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
@github-actions github-actions Bot added p: material_ui triage-design Should be looked at in design triage labels Sep 11, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated: the assert now allows non-RenderObject controllers that delegate addInkFeature to the host Material.

Comment on lines +779 to +782
if (controller is! RenderObject) {
return;
}
final paintContext = controller as RenderObject;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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;
    }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

p: material_ui triage-design Should be looked at in design triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

InkFeature public interface uses private type

1 participant