[flutter_svg] Add SvgPicture.precompiled constructor
#10255
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new convenience constructor, SvgPicture.precompiled, to simplify loading precompiled vector graphics assets. This change improves developer experience by removing the need to manually add a dependency on vector_graphics, import it, and use AssetBytesLoader. The changes include updating the README.md and example code to use the new constructor, adding a new test case for the constructor, and bumping the package version. The implementation is clean and the addition is well-documented. I have one minor suggestion regarding documentation consistency in an example file.
third_party/packages/flutter_svg/example/lib/readme_excerpts.dart
Outdated
Show resolved
Hide resolved
| /// Flutter. | ||
| /// | ||
| /// If [excludeFromSemantics] is true, then [semanticsLabel] will be ignored. | ||
| SvgPicture.precompiled( |
There was a problem hiding this comment.
Are you fine with the name precompiled? Or should we choose a different name?
| sdk: flutter | ||
| flutter_svg: | ||
| path: ../ | ||
| vector_graphics: ^1.1.13 |
There was a problem hiding this comment.
FYI: Not needed anymore
| /// Either the [width] and [height] arguments should be specified, or the | ||
| /// widget should be placed in a context that sets tight layout constraints. | ||
| /// Otherwise, the image dimensions will change as the image is loaded, which | ||
| /// will result in ugly layout changes. | ||
| /// | ||
| /// If `matchTextDirection` is set to true, the picture will be flipped | ||
| /// horizontally in [TextDirection.rtl] contexts. | ||
| /// | ||
| /// The `allowDrawingOutsideViewBox` parameter should be used with caution - | ||
| /// if set to true, it will not clip the canvas used internally to the view box, | ||
| /// meaning the picture may draw beyond the intended area and lead to undefined | ||
| /// behavior or additional memory overhead. | ||
| /// | ||
| /// A custom `placeholderBuilder` can be specified for cases where decoding or | ||
| /// acquiring data may take a noticeably long time. | ||
| /// | ||
| /// The `color` and `colorBlendMode` arguments, if specified, will be used to set a | ||
| /// [ColorFilter] on any [Paint]s created for this drawing. | ||
| /// | ||
| /// ## Assets in packages | ||
| /// | ||
| /// To create the widget with an asset from a package, the [package] argument | ||
| /// must be provided. For instance, suppose a package called `my_icons` has | ||
| /// `icons/heart.svg.vec`. | ||
| /// | ||
| /// Then to display the image, use: | ||
| /// | ||
| /// ```dart | ||
| /// SvgPicture.precompiled('icons/heart.svg.vec', package: 'my_icons') | ||
| /// ``` | ||
| /// | ||
| /// Assets used by the package itself should also be displayed using the | ||
| /// [package] argument as above. | ||
| /// | ||
| /// If the desired asset is specified in the `pubspec.yaml` of the package, it | ||
| /// is bundled automatically with the app. In particular, assets used by the | ||
| /// package itself must be specified in its `pubspec.yaml`. | ||
| /// | ||
| /// A package can also choose to have assets in its 'lib/' folder that are not | ||
| /// specified in its `pubspec.yaml`. In this case for those images to be | ||
| /// bundled, the app has to specify which ones to include. For instance a | ||
| /// package named `fancy_backgrounds` could have: | ||
| /// | ||
| /// ```none | ||
| /// lib/backgrounds/background1.svg.vec | ||
| /// lib/backgrounds/background2.svg.vec | ||
| /// lib/backgrounds/background3.svg.vec | ||
| ///``` | ||
| /// | ||
| /// To include, say the first image, the `pubspec.yaml` of the app should | ||
| /// specify it in the assets section: | ||
| /// | ||
| /// ```yaml | ||
| /// assets: | ||
| /// - packages/fancy_backgrounds/backgrounds/background1.svg.vec | ||
| /// ``` | ||
| /// | ||
| /// The `lib/` is implied, so it should not be included in the asset path. | ||
| /// | ||
| /// | ||
| /// See also: | ||
| /// | ||
| /// * <https://flutter.io/assets-and-images/>, an introduction to assets in | ||
| /// Flutter. | ||
| /// | ||
| /// If [excludeFromSemantics] is true, then [semanticsLabel] will be ignored. |
There was a problem hiding this comment.
FYI: Copied this part from SvgPicture.asset (only changed file names from NAME.svg to NAME.svg.vec).
| @Deprecated('Use colorFilter instead.') ui.Color? color, | ||
| @Deprecated('Use colorFilter instead.') |
There was a problem hiding this comment.
Should we add the @Deprecated parameters to the new constructor, or leave them out?
|
@domesticmouse Thanks for your quick review! Do you have feedback for the questions that I posted? |
|
PTAL @stuartmorgan-g |
2 participants
flutter_svgallows precompiled SVG files, see Precompiling and Optimizing SVGs.However, as a developer, you have to add three things to your code:
vector_graphicsas a direct dependencyimport 'package:vector_graphics/vector_graphics.dart';importAssetBytesLoaderclass.With
SvgPicture.precompiled(String assetName, [...])a developer of the package can use precompiled SVG files without the three steps above.Closes flutter/flutter#177068
Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.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 ↩3