This feels a bit strange since it's very different from what TextStyle does (either color or paint, not both), and that can be surprising. IMO this should probably be a separate constructor, or follow the TextStyle convention and add an assert, so the API stays consistent.

I’m confused about what ‘follow the TextStyle convention’ means. Could you please provide more explanation?

Ah sorry so https://main-api.flutter.dev/flutter/painting/TextStyle/color.html says:

If foreground is specified, this value must be null. The color property is shorthand for Paint()..color = color.

and in the constructor implementation it asserts: assert(color == null || foreground == null, _kColorForegroundWarning).

Ok but the color property can still be provided, but it will override the color attribute set in the foreground. For example, when color is set to Colors.red and foreground is set to Paint()..color = Colors.blue, the final color will be red.

Is there a reason color has a higher priority than foreground.color, and not the other way around? I think putting multiple parameters that refer to the same thing in the same API usually makes the API less self-explanatory.

Sounds reasonable. I will give foreground.color higher priority with the following code:

if (foreground!.color == null) {
  foreground!.color = iconColor
}

Sorry my point was that color and foreground.color should not be specified at the same time, it should be considered an incorrect use of the API if they are both set. The reasons are:

• Giving color or foreground.color a higher priority than the other is a bit arbitrary IMO,

• I think most of the time when developers write: Icon(color: ..., foreground: Paint()..color = ...), it's by mistake. It saves their time to debug this kind of error if we assert that not both of them are set, or if we prevent them from doing this at compile time (by introducing another constructor).

Oh finally got it! Thanks for your patience. Sounds like writing another constructor like:

// this constructor will not provide color, avoid developers making mistake
Icon.withForeground({
  ...,
})

is overkill for me, so I will try using assertion.

@LongCatIsLooong, I encountered an issue:

If a developer only passes in foreground, how should the foreground color be determined? It is unclear whether foreground has a set color (since the default color of Paint is opaque black, not null). However, completely relying on the color provided by foreground would mean that the developer has to inject the value from IconTheme every time they pass in foreground.

I believe we should not use the color from foreground and instead directly use the final computed iconColor in Icon to override the Paint's color.

how should the foreground color be determined?

I would think it's just either paint or color (see TextStyle's constructor https://main-api.flutter.dev/flutter/painting/TextStyle/TextStyle.html).

However, completely relying on the color provided by foreground would mean that the developer has to inject the value from IconTheme every time they pass in foreground

Do you mean that we need an IconTheme.foreground?