Conversation
| axisNameWidget: Text(analysisFunction | ||
| .metric.abbreviatedLocalizedName), | ||
| axisNameWidget: Text( | ||
| "${analysisFunction.metric.abbreviatedLocalizedName} ${analysisFunction.metric.units != "" ? "(${analysisFunction.metric.units})" : ""}"), |
There was a problem hiding this comment.
If there are no units for a metric, it would probably make more sense to store it as null instead of an empty string. I also generally recommend avoiding nesting ternaries like this.
There's also a trailing space if there are no units.
| .valueVizualizationBuilder(value), | ||
| ), | ||
| ), | ||
| maxIncluded: true, |
There was a problem hiding this comment.
true is the default for maxIncluded, so this change doesn't affect the behavior. Were you looking to disable it? That might make the labels more readable for users
| child: Text(numToStringRounded(value)), | ||
| ); | ||
| }, | ||
| reservedSize: 50), |
There was a problem hiding this comment.
We can probably reduce the reservedSize now that unit names are taking up less space
lib/metrics.dart
Outdated
| bool hideFlag; | ||
| double? max; | ||
|
|
||
| String units; |
| String units; | ||
| String path; | ||
|
|
||
| String Function(dynamic)? valueToString; |
There was a problem hiding this comment.
Were we going to make this add the units on by default?
| localizedName: "Scoring Rate (Fuel / Second)", | ||
| abbreviatedLocalizedName: "Scoring Rate", | ||
| valueToString: ((p0) => "$p0 bps"), | ||
| units: " bps", |
There was a problem hiding this comment.
Why is this " bps" while feeding rate is "bps"?
MangoSwirl
left a comment
MangoSwirl
left a comment
There was a problem hiding this comment.
Yeah this is a much better way to do it. Looks good!
2 participants
No description provided.