[flutter_svg] add imageBuilder property to SvgPicture#11615
[flutter_svg] add imageBuilder property to SvgPicture#11615suojae wants to merge 4 commits intoflutter:mainfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds an imageBuilder property to the SvgPicture widget, enabling custom wrapping of successfully loaded SVGs. The implementation includes a new SvgImageWidgetBuilder typedef, updates to all SvgPicture constructors, and a version bump to 2.3.0. The vector_graphics dependency is also updated to 1.2.0, supported by new unit tests for the imageBuilder functionality. I have no feedback to provide.
|
Can you spend some more time explaining in the description why you think we need this API. What problem is it solving and what alternatives did you consider? |
gaaclarke
left a comment
There was a problem hiding this comment.
(requested more information in description above)
|
Oh, sorry for the thin description. I've updated the PR description to explain why this API is needed, what problem it solves, and the alternatives I considered. |
gaaclarke
left a comment
There was a problem hiding this comment.
Okay, thanks for expanding on why we are doing this. While there was technically a way to achieve this before the PR, it required a lot of boilerplate code (like ByteLoader subclass). It's unreasonable to expect all users to implement that. This is consistent with existing APIs and sounds good to me.
My only concern is I believe the new tests operate with the svg cache so there can be bleeding state between tests. I think it would be prudent to clear the cache between tests:
setUp(() {
svg.cache.clear();
});|
Also a friendly reminder: when you want me to re-review things, please press the "re-request review" button otherwise I may miss it. |
related with flutter/flutter#182635
This PR exposes the
imageBuildersupport added toVectorGraphicin #11094 throughSvgPicture.SvgPicturealready supportsplaceholderBuilderanderrorBuilder, but it did not provide a success-state builder. Without this API, callers have to wrapSvgPictureexternally, which also wraps the placeholder and error states.Adding
imageBuilderlets callers decorate or wrap only the successfully loaded SVG, while keeping loading and error UI separate.Alternatives considered
imageBuilderbecause it follows the success-state wrapping pattern used by widgets likeCachedNetworkImageChanges
SvgImageWidgetBuilder.imageBuilderto allSvgPictureconstructors.imageBuildertocreateCompatVectorGraphic.flutter_svgto 2.3.0 andvector_graphicsto ^1.2.0.Pre-Review Checklist