Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

documentation: Adds migration advice, removes no-op deprecations #861

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

documentation: Adds migration advice, removes no-op deprecations #861

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3cadc1
documentation: Add link to migration guide and remove no-op deprecati…
dkbast Feb 13, 2023
9550aa0
improve punctuation
dkbast Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

Draw SVG files using Flutter.

If you are migrating from flutter_svg 1.x to 2.x, please take a look at the [migration document](https://github.com/dnfield/flutter_svg/blob/master/vector_graphics.md)

## Getting Started

This package provides a wrapper around Dart implementations of SVG parsing,
Expand Down
87 changes: 48 additions & 39 deletions lib/svg.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,13 @@ final Svg svg = Svg._();
class Svg {
Svg._();

/// A global override flag for [SvgPicture.cacheColorFilter].
///
/// If this is null, the value in [SvgPicture.cacheColorFilter] is used. If it
/// is not null, it will override that value.
@Deprecated('This no longer does anything.')
bool? cacheColorFilterOverride;
Copy link
Owner

Choose a reason for hiding this comment

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

Please don't remove these. There are usages of this API in some customers and removing them makes it much harder to migrate those customers to the new version.

Copy link
Author

Choose a reason for hiding this comment

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

Especially with the doc comment still pointing towards this "doing something" this is quite confusing.


/// The cache instance for decoded SVGs.
final Cache cache = Cache();
}

// ignore: avoid_classes_with_only_static_members
/// Deprecated class, will be removed, does not do anything.
@Deprecated('This feature does not do anything anymore.')
class PictureProvider {
/// Deprecated, use [svg.cache] instead.
@Deprecated('Use svg.cache instead.')
static Cache get cache => svg.cache;
}

/// A widget that will parse SVG data for rendering on screen.
class SvgPicture extends StatelessWidget {
/// Instantiates a widget that renders an SVG picture using the `pictureProvider`.
/// Instantiates a widget that renders an SVG picture using a [BytesLoader].
///
/// Either the [width] and [height] arguments should be specified, or the
/// widget should be placed in a context that sets tight layout constraints.
Expand Down Expand Up @@ -87,8 +71,6 @@ class SvgPicture extends StatelessWidget {
this.semanticsLabel,
this.excludeFromSemantics = false,
this.theme = const SvgTheme(),
@deprecated Clip? clipBehavior,
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto here. I'd like to keep these deprecated properties around for a while to ease migration.

Copy link
Author

Choose a reason for hiding this comment

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

Hmm ... I would actually argue that they actually hinder migration, since by keeping them as noop it's not safe to assume that we can "ignore" deprecation warnings as in this case the deprecation already happened.

@deprecated bool cacheColorFilter = false,
}) : super(key: key);

/// Instantiates a widget that renders an SVG picture from an [AssetBundle].
Expand Down Expand Up @@ -182,11 +164,16 @@ class SvgPicture extends StatelessWidget {
this.semanticsLabel,
this.excludeFromSemantics = false,
this.theme = const SvgTheme(),

/// The [ColorFilter] to apply to the picture. An easy way to add color.
/// e.g. `ColorFilter.mode(Colors.red, BlendMode.srcIn)`
Comment on lines +168 to +169
Copy link
Owner

Choose a reason for hiding this comment

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

Here and elsewhere: docs should contain complete sentences thatstart with a capital letter and end with a period.

It would be great to expand this a bit to mention that it is also possible to use the ColorMapper property which avoids filtering at runtime and instead replaces colors at parse time.

Copy link
Author

Choose a reason for hiding this comment

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

Hey - thats sounds great! I'm currently not more knowledgable to suggest anything more than what I needed to do my migration, but please feel free to improve upon this - the PR is open for Maintainer edits :)

ui.ColorFilter? colorFilter,
@deprecated ui.Color? color,
@deprecated ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
@deprecated Clip? clipBehavior,
@deprecated bool cacheColorFilter = false,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
dkbast marked this conversation as resolved.
Show resolved Hide resolved
ui.Color? color,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
}) : bytesLoader = SvgAssetLoader(
assetName,
packageName: package,
Expand Down Expand Up @@ -239,13 +226,18 @@ class SvgPicture extends StatelessWidget {
this.matchTextDirection = false,
this.allowDrawingOutsideViewBox = false,
this.placeholderBuilder,

/// The [ColorFilter] to apply to the picture. An easy way to add color.
/// e.g. `ColorFilter.mode(Colors.red, BlendMode.srcIn)`
ui.ColorFilter? colorFilter,
@deprecated ui.Color? color,
@deprecated ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.Color? color,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
this.semanticsLabel,
this.excludeFromSemantics = false,
@deprecated Clip? clipBehavior,
@deprecated bool cacheColorFilter = false,
this.theme = const SvgTheme(),
}) : bytesLoader = SvgNetworkLoader(url, headers: headers, theme: theme),
colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode),
Expand Down Expand Up @@ -291,14 +283,21 @@ class SvgPicture extends StatelessWidget {
this.matchTextDirection = false,
this.allowDrawingOutsideViewBox = false,
this.placeholderBuilder,

/// The [ColorFilter] to apply to the picture. An easy way to add color.
/// e.g. `ColorFilter.mode(Colors.red, BlendMode.srcIn)`
ui.ColorFilter? colorFilter,
@deprecated ui.Color? color,
@deprecated ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.Color? color,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
this.semanticsLabel,
this.excludeFromSemantics = false,
this.theme = const SvgTheme(),
@deprecated Clip? clipBehavior,
@deprecated bool cacheColorFilter = false,
@Deprecated('This has property has been removed')
Clip? clipBehavior,
}) : bytesLoader = SvgFileLoader(file, theme: theme),
colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode),
super(key: key);
Expand Down Expand Up @@ -340,14 +339,19 @@ class SvgPicture extends StatelessWidget {
this.matchTextDirection = false,
this.allowDrawingOutsideViewBox = false,
this.placeholderBuilder,

/// The [ColorFilter] to apply to the picture. An easy way to add color.
/// e.g. `ColorFilter.mode(Colors.red, BlendMode.srcIn)`
ui.ColorFilter? colorFilter,
@deprecated ui.Color? color,
@deprecated ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.Color? color,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
this.semanticsLabel,
this.excludeFromSemantics = false,
this.theme = const SvgTheme(),
@deprecated Clip? clipBehavior,
@deprecated bool cacheColorFilter = false,
}) : bytesLoader = SvgBytesLoader(bytes, theme: theme),
colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode),
super(key: key);
Expand Down Expand Up @@ -389,14 +393,19 @@ class SvgPicture extends StatelessWidget {
this.matchTextDirection = false,
this.allowDrawingOutsideViewBox = false,
this.placeholderBuilder,

/// The [ColorFilter] to apply to the picture. An easy way to add color.
/// e.g. `ColorFilter.mode(Colors.red, BlendMode.srcIn)`
ui.ColorFilter? colorFilter,
@deprecated ui.Color? color,
@deprecated ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.Color? color,
@Deprecated('Use colorFilter: ColorFilter.mode(color, blendMode) instead '
'- default BlendMode is BlendMode.srcIn )')
ui.BlendMode colorBlendMode = ui.BlendMode.srcIn,
this.semanticsLabel,
this.excludeFromSemantics = false,
this.theme = const SvgTheme(),
@deprecated Clip? clipBehavior,
@deprecated bool cacheColorFilter = false,
}) : bytesLoader = SvgStringLoader(string, theme: theme),
colorFilter = colorFilter ?? _getColorFilter(color, colorBlendMode),
super(key: key);
Expand Down
17 changes: 16 additions & 1 deletion vector_graphics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ SVGs, and the way that Flutter ties together byte acquisition and image decoding
makes things a bit complicated. Instead, `vector_graphics` introduces the
concept of a `BytesLoader` class, which knows how to obtain encoded bytes to
feed to the runtime via an asynchronous method that optionally receives a
`BuildContext`. See the `loaders.dart` file in this package for examples.
`BuildContext`. See the `loaders.dart` file in this package for examples.

As of this writing, the optimizations that are available include:

Expand Down Expand Up @@ -167,3 +167,18 @@ await tester.pump();

to make sure that the image decode(s) finish and the placeholder widget is
replaced with the actual picture.

### Widget testing

For creating matchers for an svg asset you can use the following as suggested in [#852](https://github.com/dnfield/flutter_svg/issues/852):

```dart
final findAsset = find.byWidgetPredicate(
(widget) =>
widget is SvgPicture &&
(widget.bytesLoader as SvgAssetLoader) // Here!
.assetName
.compareTo('assets/flutter.svg') ==
0,
);
```