color in SvgPicture

[mahdiqdi]

i used color property to change svg color.but now its deprected.how should i change my svg color

[jannisnikoy]

@mahdiqdi the docs seem to suggest using the colorFilter property instead. Wish color: remained, even if under the hood it'd use colorFilter. Feel like this is adding a lot of unnecessary code, especially when using lots of svgs 🤷

colorFilter: ColorFilter.mode(Colors.red, BlendMode.srcIn)

[mahdiqdi]

right now i am using lots of svgs as icon in my flutter app.does colorFilter decrease my performance or not?
@jannisnikoy

[felipecastrosales]

Yes, docs suggest colorFilter. But I think there could be a better Migration Guide.

[dnfield]

I would be happy to review any PRs that update the docs and/or read me to help people out with this.

Part of what I'm trying to encourage users of this API to think about is what kind of color filter they want, or whether they want to try to use the ColorMapper API instead.

[hasanmhallak]

@dnfield but why not leaving it the way it is? with these changes a lot of users will have so much code to write refactoring large code bases let alone multiple projects.

It is a good idea to encourage users to think of color filter, but this is what optional parameters all about in the first place right?

[dnfield]

The property is deprecated but still may be used. There is an easy path to migration forward to a more flexible API.

[hasanmhallak]

Thanks for the reply, I know it can still be used, but deprecation means eventually this property will be removed.

I'm just saying this is adding a lot of unnecessary code, especially when using lots of svgs. Not all developers are using the colorFilter or need to add a matrix based filters. but if someone would, a colorFilter property should override the color property.

[dnfield]

Deprecated means there's a better option. I am in no rush to remove this and am willing to keep it around for a long time to let people migrate :)

[petro-i]

Why is the development of this package makes life more difficult?
Color parameter was so easy to use, but now we have to add one more import at least to have the same functionality. I think color parameter deprecation was an error. Use case of simple "color" parameter is a lot more useful then complex "colorFilter" which is not easy to use and understand how it works in case we neeed just paint svg with single color.

[felipecastrosales]

@petro-i, if you need to use only one color, use it like this:

colorFilter: ColorFilter.mode(YOURCOLOR, BlendMode.srcIn)

Unfortunately that's it. But I understand the maintainer's explanation.

[HappyMakadiyaS]

Why color is not nullable type in ColorFilter.mode?
In my use case, having condition-based color assignment that can be nullable in many cases and earlier with colour attribute it was handled nicely. Now I have to put unnecessary null check conditions over ColorFilter.

[dnfield]

I'm going to close this issue.

The goal of deprecating color is to get users of this library to think more seriously about how and whether a color filter is right for them. Color filtering is generally more expensive than color mapping, especially for some blend modes. Color filtering also sometimes is hard to reason about if you're not familiar with the blend modes and how they work, or when and whether you should use a color matrix instead of a Porter-Duff blend.

As mentioned, I have no plans to remove the color parameter. If you want to use it, keep using it. But I think deprecating it achieves the goal of getting users to think a bit more about what kind of color filtering they want and whether they want it at all. I'd be open to improvements in documentation around this.

[petro-i]

to get users of this library to think more seriously about how and whether a color filter is right for them
sorry, but you are not teacher and we are not in school.
The idea against deprecating color is just so filter is harder to configure in everyday usage then just color, people need color in 99% of cases, it is just not time-optimal to use filter instead of color, it needs more time so in fact in 99% cases we are wasting time configuring filter == you force people to waste time using this library.

[dnfield]

I'm a bit stumped here.

I've marked the property deprecated because people file bugs about it because it doesn't work the way their mental model of it wants it to work, but the mental model people have about it is often incorrect (because honestly it's not super easy to reason about outside of very simple cases). I do recognize the property has value for a lot of people using it in simple cases, and have said a few times that I don't intend to remove it anytime soon.

Instead of the deprecated property, I'd like consumers of this library to decide about whether they really want a ColorFilter or instead a ColorMapper - and if they do want a ColorFilter, theyshould think a bit about which color filtering algorithm is the right one for their use case.

This isn't so much about whether I'm a teacher or not, but I'm writing an API here and I've always felt this particular part of the API was not effectively communicating what it does to consumers - who see the color property and ignore the blendMode property and either get confused or start randomly trying different blend modes, when in fact none of the Porter-Duff blend modes will solve their problem since what they really want is a matrix based color filter on the input.

[hongfeiyang]

is there an example of how to use the ColorMapper API? It is an abstract class with no concrete subclass, so I am curious to know which subclass of it is currently being used.

[Maatteogekko]

@hongfeiyang you are expected to implement your own class. This is how I have done it.

class MyColorMapper implements ColorMapper {
  const MyColorMapper({
    required this.baseColor,
    this.accentColor,
  });

  static const _rawBaseColor = Color(0xFF293540);
  static const _rawAccentColor = Color(0xFFFFFFFF);

  final Color baseColor;
  final Color? accentColor;

  @override
  Color substitute(String? id, String elementName, String attributeName, Color color) {
    if (color == _rawBaseColor) return baseColor;

    final accentColor = this.accentColor;
    if (accentColor != null && color == _rawAccentColor) return accentColor;

    return color;
  }
}

This is an example of the svgs I use with this mapper

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24">
  <g stroke-linecap="round" stroke-linejoin="round" stroke-width="1.5">
    <path fill="#293540" stroke="#293540" d="M22 12c0 5.52-4.48 10-10 10S2 17.52 2 12 6.48 2 12 2s10 4.48 10 10Z" />
    <path fill="none" stroke="#fff" d="M8 12h8" />
  </g>
</svg>

In short, I know that the svg elements can have two colors, and map them to the colors I actually want in the mapper

[MubeenNaeem]

To all the people saying they will have to refactor a lot of code, I will suggest you to create a reusable widget for Svg Icons like this:

import 'package:flutter/material.dart';
import 'package:flutter_svg/flutter_svg.dart';

class SvgIcon extends StatelessWidget {
  final String icon;
  final double size;
  final Color color;

  const SvgIcon({
    Key? key,
    required this.icon,
    required this.color,
    this.size = 24,
  }) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return SvgPicture.asset(
      'assets/$icon.svg',
      colorFilter: ColorFilter.mode(color, BlendMode.srcIn),
      height: size,
    );
  }
}

[ollyde]

@petro-i, if you need to use only one color, use it like this:

colorFilter: ColorFilter.mode(YOURCOLOR, BlendMode.srcIn)

Unfortunately that's it. But I understand the maintainer's explanation.

It's actually broken though, I'm getting funky colors using the same color hex.

[ChauCM]

I would prefer you keep the color options but the user can still have the colorFilter field.

Please think of it as

Container(
    color: Colors.white
  )

vs

Container(
  decoration: BoxDecoration(
    color: Colors.white,
  ),

The flutter team still leaves us the option to quickly add color to the Container instead of going the complicated way.

[da-nish]

To all the people who don't like writing ColorFilter.mode(this, BlendMode.srcIn); every place

SvgPicture.asset(
   "image",
   colorFilter: Colors.red.toColorFilter,
),

extension ToColorFilter on Color {
   ColorFilter? get toColorFilter {
      return ColorFilter.mode(this, BlendMode.srcIn);
  }
}

[felipecastrosales]

@da-nish this is a good suggestion, but using extensions methods the widget can't be const.

[ollyde]

Extensions still don't import or work so well, would be nice to have this has a convenience method.

[KlGleb]

Deprecated means that the parameter can be removed. Even though the author has written several times that he is not going to remove this method, ide will always highlight the use of this parameter, and it looks very dirty -- especially for people like me who want to keep warnings clean. So, in my opinion, in this case the author could have just clarified the documentation instead of labeling the method deprecated.

At the same time, constantly duplicating colorFilter: ColorFilter.mode(YOURCOLOR, BlendMode.srcIn) also looks bad in terms of code readability. Besides, this code is much harder to write: personally, I have to go to the documentation every time to remember what exactly needs to be passed there. In other similar widgets, in particular, in Icon, it is the color parameter that is used.

To summarize all of the above, the colorFilter parameter has the following disadvantages:

1. Code duplication
2. Poor readability
3. Complexity of writing
4. Not intuitive to use

The only advantage here is the possibility to use different ColorFilters, which is cool and may be useful in some not-very frequent cases. I regret that the author decided that this advantage is more important than all the disadvantages.

I guess the best way to solve all the problems at the moment is to write a wrapper over SvgPicture and use it.