CupertinoDynamicColor improvements#44317
Conversation
fluttergithubbot
added
a: tests
| @override | ||
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | ||
| super.debugFillProperties(properties); | ||
| const CupertinoTextThemeData defaultData = CupertinoTextThemeData(); |
There was a problem hiding this comment.
Getting error • Evaluation of this constant expression throws an exception • packages/flutter/lib/src/cupertino/text_theme.dart:255:48 • const_eval_throws_exception but the analyzer won't let me change to final either.
There was a problem hiding this comment.
Is it because you're missing required parameters (this assertion https://github.com/flutter/flutter/pull/44317/files#diff-6cb3174a51ae9b800fc6377dfc24234cR161)?
There was a problem hiding this comment.
primaryColor has a default value CupertinoColors.systemBlue. The weird thing is I'm able to compile apps and they run without throwing, despite this analyzer error.
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
This all looks pretty slick.
One the face of it, I'm not sure what https://github.com/flutter/flutter/pull/44317/files#r343507022 is all about.
| properties.add(DiagnosticsProperty<Element>('last resolved', _debugResolveContext)); | ||
| } | ||
|
|
||
| /// Asserts that the |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM
Pretty cool, I didn't realize how to set up debug things like that.
| @override | ||
| void debugFillProperties(DiagnosticPropertiesBuilder properties) { | ||
| super.debugFillProperties(properties); | ||
| const CupertinoTextThemeData defaultData = CupertinoTextThemeData(); |
There was a problem hiding this comment.
Is it because you're missing required parameters (this assertion https://github.com/flutter/flutter/pull/44317/files#diff-6cb3174a51ae9b800fc6377dfc24234cR161)?
5 participants
Description
This PR is an attempt at making the
CupertinoDynamicColormore debuggable.If a CupertinoDynamicColor is used to paint before being resolved, it should throwNot feasible because that will break people that are currently usingCupertinoColors.activeBlueas a concrete color. But this assert (now removed) did catch a few unresolved color usages in the framework and tests.debugLabelfield toCupertinoDynamicColor.Screenshots
Inspecting a dynamic color in the widget inspector.
Clicking on "last resolved" will link you to that element, thanks to the dev tool.
Enhanced
CupertinoThemedisplayRelated Issues
#35541
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?