Add `WidgetStateProperty` example and tests for it. #155315

ksokolovskyi · 2024-09-17T18:27:12Z

This PR contributes to #155313

Description

Adds example for WidgetStateProperty
Adds tests for examples/api/lib/widgets/widget_state/widget_state_property.0.dart

Pre-launch Checklist

I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
I read the [Tree Hygiene] wiki page, which explains my responsibilities.
I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
I signed the [CLA].
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is [test-exempt].
I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
All existing and new tests are passing.

nate-thegrate

Thanks very much for adding this!

I think the conversation from #89127 is relevant here—the WidgetStateProperty.resolveWith() constructor unfortunately doesn't support stable equality checks, so in some scenarios it can lead to a lot of unnecessary rebuilding.

Additionally, the callback it uses is kind of dense and unintuitive:

WidgetStateProperty.resolveWith((states) {
  if (states.contains(WidgetState.hovered) || states.contains(WidgetState.pressed)) {
    return Colors.blue;
  }
  return Colors.black;
})

I was recently able to get #154695 merged, so now the .fromMap() constructor supports accurate equality checks. It's also quite a bit more concise:

WidgetStateProperty.fromMap({
  WidgetState.hovered | WidgetState.pressed: Colors.blue,
  WidgetState.any: Colors.black,
})

Perhaps we could help new developers avoid the pitfalls of "broken ThemeData equality" by promoting this constructor in the example file.

Let me know what you think :)

ksokolovskyi · 2024-09-18T09:01:00Z

Hi @nate-thegrate

Thanks for the review and your work on WidgetStateProperty.fromMap!

I applied your suggestions in the latest commit, could you please take a look again?

Additionally, I have taken a look at the implementation of the fromMap and noticed that it relies on the order of the map entries. When using the regular map I didn't notice any bugs, as the order of the entries equals the order in the code. But when using e.g. HashMap (I don't know why someone may decide to use it in WidgetStateProperty, but still) the order of the entries may change from launch to launch which may cause some hard-to-spot bugs. Don't you think that it is worth mentioning that users must pass the regular map into the fromMap?

To reproduce the behavior you can modify the widget_state_property.0.dart code with the following foregroundColor and run widget_state_property.0_test.dart tests a couple of times.

foregroundColor: WidgetStateProperty<Color>.fromMap(
  HashMap<WidgetStatesConstraint, Color>.from(
    <WidgetStatesConstraint, Color>{
      WidgetState.pressed | WidgetState.hovered | WidgetState.focused:
          Colors.blue,
      WidgetState.any: Colors.red,
    },
  ),
),

The recording of the mentioned behavior:

widget_state_property_from_map.mov

nate-thegrate · 2024-09-18T15:04:06Z

When using the regular map I didn't notice any bugs, as the order of the entries equals the order in the code. But when using e.g. HashMap (I don't know why someone may decide to use it in WidgetStateProperty, but still) the order of the entries may change from launch to launch which may cause some hard-to-spot bugs. Don't you think that it is worth mentioning that users must pass the regular map into the fromMap?

I also don't know why someone would go out of their way to make their map not work, but this is a good point regardless!

As luck would have it, I have a PR open to configure each fromMap constructor so it inherits from the same class. I just pushed a commit there that adds an assert to that base class. Thanks very much for helping me out here!

edit: had to revert the commit since it was causing errors ☹️
(My guess is that depending on platform, LinkedHashMap sometimes counts as a HashMap)

ksokolovskyi · 2024-09-18T15:07:10Z

When using the regular map I didn't notice any bugs, as the order of the entries equals the order in the code. But when using e.g. HashMap (I don't know why someone may decide to use it in WidgetStateProperty, but still) the order of the entries may change from launch to launch which may cause some hard-to-spot bugs. Don't you think that it is worth mentioning that users must pass the regular map into the fromMap?

I also don't know why someone would go out of their way to make their map not work, but this is a good point regardless!

As luck would have it, I have a PR open to configure each fromMap constructor so it inherits from the same class. I just pushed a commit there that adds an assert to that base class. Thanks very much for helping me out here!

Look great!

nate-thegrate

Thanks for your work here!

Feel free to take a look at the optional suggestion below if you'd like :)

examples/api/lib/widgets/widget_state/widget_state_property.0.dart

examples/api/test/widgets/widget_state/widget_state_property.0_test.dart

ksokolovskyi · 2024-09-18T15:14:20Z

Thanks for your work here!

Feel free to take a look at the optional suggestion below if you'd like :)

Thanks for the review!
Nice suggestions, so I applied them in the latest commit.

Apply suggestions. Co-authored-by: Nate Wilson <[email protected]>

bleroux

Thanks for adding this example and its tests.
Just some minor comments.

examples/api/test/widgets/widget_state/widget_state_property.0_test.dart

ksokolovskyi · 2024-09-23T14:26:25Z

@bleroux thanks a lot for the review!
I applied your suggestions in the latest commit.
Could you please review PR again?

bleroux

LGTM! Thanks for your contribution 🙏

Just one minor change required on the documentation and an optional one related to format.

bleroux · 2024-09-23T14:45:47Z

packages/flutter/lib/src/widgets/widget_state.dart

 /// material state properties when their value is needed.
 ///
+/// {@tool dartpad}
+/// This example shows how you can override the default text and icon


Just noticed the "you", from the style guide it should be avoided.
Maybe something such as "This example shows how the default text and icon color (the "foreground color") of a [TextButton] can be overridden with a [WidgetStateProperty]."

bleroux · 2024-09-23T14:47:39Z

examples/api/test/widgets/widget_state/widget_state_property.0_test.dart

+    final TestGesture gesture =
+        await tester.createGesture(kind: PointerDeviceKind.mouse);


It looks like autoformat was applied there, feel free to change it or not.

ksokolovskyi · 2024-09-23T17:21:57Z

@bleroux thanks for the additional suggestions, I have just applied them in the latest commit.

bleroux

LGTM!

…flutter#155315)

Roll Flutter from 4ca51a1 to 538e742 (40 revisions) flutter/flutter@4ca51a1...538e742 2024-09-25 [email protected] Marks Linux build_aar_module_test to be unflaky (flutter/flutter#155349) 2024-09-25 [email protected] Roll Packages from 4926c0f to 7da2374 (3 revisions) (flutter/flutter#155701) 2024-09-25 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Normalize TabBarTheme (#155476)" (flutter/flutter#155698) 2024-09-25 [email protected] Normalize TabBarTheme (flutter/flutter#155476) 2024-09-25 [email protected] increase both linux & windows tool integration test shards (flutter/flutter#155631) 2024-09-25 [email protected] Roll Flutter Engine from c7cd559e483b to d6d5fdba6ae1 (1 revision) (flutter/flutter#155693) 2024-09-25 [email protected] Add WidgetStateBorderSide example and tests for it. (flutter/flutter#155559) 2024-09-25 [email protected] Roll Flutter Engine from b9dd7a39dd58 to c7cd559e483b (1 revision) (flutter/flutter#155686) 2024-09-25 [email protected] Roll Flutter Engine from 87c1667dfd1e to b9dd7a39dd58 (1 revision) (flutter/flutter#155684) 2024-09-25 [email protected] Roll Flutter Engine from 05211f9d2267 to 87c1667dfd1e (1 revision) (flutter/flutter#155681) 2024-09-25 [email protected] Roll Flutter Engine from 8d1eb7410b49 to 05211f9d2267 (1 revision) (flutter/flutter#155672) 2024-09-25 [email protected] Roll Flutter Engine from ad3dd0df0fe7 to 8d1eb7410b49 (2 revisions) (flutter/flutter#155662) 2024-09-25 [email protected] Roll Flutter Engine from 746ce6124844 to ad3dd0df0fe7 (2 revisions) (flutter/flutter#155653) 2024-09-25 [email protected] Add PrivacyInfo.xcprivacy to macOS plugin template (flutter/flutter#155570) 2024-09-25 [email protected] Roll Flutter Engine from 559f2ff31c74 to 746ce6124844 (14 revisions) (flutter/flutter#155648) 2024-09-25 [email protected] fix `SearchAnchor` disposing `SearchController` while it is still used (flutter/flutter#155219) 2024-09-25 [email protected] Roll pub packages (flutter/flutter#155640) 2024-09-24 [email protected] Preserve transform when using *Gradient:withOpacity (flutter/flutter#154908) 2024-09-24 [email protected] fixed keyboardDismissBehavior on scroll without a drag (flutter/flutter#154675) 2024-09-24 [email protected] Roll Flutter Engine from 7cd3d0b1bb2e to 559f2ff31c74 (3 revisions) (flutter/flutter#155629) 2024-09-24 [email protected] Roll Flutter Engine from 2a13c3a27e1f to 7cd3d0b1bb2e (4 revisions) (flutter/flutter#155625) 2024-09-24 [email protected] Misc docs cleanup and fixes (flutter/flutter#155501) 2024-09-24 [email protected] Roll Flutter Engine from dc44f95b7027 to 2a13c3a27e1f (1 revision) (flutter/flutter#155619) 2024-09-24 [email protected] Roll Flutter Engine from 2745b8797025 to dc44f95b7027 (1 revision) (flutter/flutter#155616) 2024-09-24 [email protected] Roll Flutter Engine from 8a54cc56d4b9 to 2745b8797025 (2 revisions) (flutter/flutter#155610) 2024-09-24 [email protected] Roll Flutter Engine from c07812775255 to 8a54cc56d4b9 (2 revisions) (flutter/flutter#155607) 2024-09-24 [email protected] Roll Packages from 9de72be to 4926c0f (4 revisions) (flutter/flutter#155605) 2024-09-24 [email protected] Fix some broken links in DAP readme (flutter/flutter#155600) 2024-09-24 [email protected] Roll Flutter Engine from 22e4f015cc99 to c07812775255 (2 revisions) (flutter/flutter#155599) 2024-09-24 [email protected] [flutter_tools] Fix encoded stderr in "dart.log" from debug adapter to client (flutter/flutter#155249) 2024-09-24 [email protected] Roll Flutter Engine from 309468cfd1bb to 22e4f015cc99 (2 revisions) (flutter/flutter#155591) 2024-09-24 [email protected] [native assets] Roll dependencies (flutter/flutter#155432) 2024-09-24 [email protected] Roll Flutter Engine from 4013dc28a48b to 309468cfd1bb (2 revisions) (flutter/flutter#155588) 2024-09-24 [email protected] Roll Flutter Engine from 8a5af19a43f3 to 4013dc28a48b (1 revision) (flutter/flutter#155585) 2024-09-24 [email protected] Roll Flutter Engine from 95c5a0940ad9 to 8a5af19a43f3 (10 revisions) (flutter/flutter#155583) 2024-09-24 [email protected] Add `WidgetStateProperty` example and tests for it. (flutter/flutter#155315) 2024-09-24 [email protected] Redo flutter engine flutter autoroll bd3d1990 485b 419c 8c55 b27e3eeb15ed 1727117767 (flutter/flutter#155579) 2024-09-23 [email protected] Roll Flutter Engine from 61f0a3fbabbe to 9bb0ece79ae2 (2 revisions) (flutter/flutter#155549) 2024-09-23 [email protected] Assert macOS framework artifact contains xcprivacy manifest (flutter/flutter#155189) 2024-09-23 [email protected] Roll Packages from f54fe93 to 9de72be (1 revision) (flutter/flutter#155540) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. ...

This PR contributes to flutter#155313 ### Description - Adds example for `WidgetStateProperty` - Adds tests for `examples/api/lib/widgets/widget_state/widget_state_property.0.dart`

…flutter#155315)

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Sep 17, 2024

nate-thegrate self-requested a review September 18, 2024 03:26

nate-thegrate reviewed Sep 18, 2024

View reviewed changes

ksokolovskyi requested a review from nate-thegrate September 18, 2024 09:01

nate-thegrate approved these changes Sep 18, 2024

View reviewed changes

examples/api/lib/widgets/widget_state/widget_state_property.0.dart Show resolved Hide resolved

examples/api/test/widgets/widget_state/widget_state_property.0_test.dart Outdated Show resolved Hide resolved

ksokolovskyi and others added 4 commits September 22, 2024 09:14

Add example for WidgetStateProperty.

b6ddaef

Add tests for WidgetStateProperty example.

1b5f94c

Replace WidgetStateProperty.resolveWith by WidgetStateProperty.fromMap

c748638

Apply suggestions from code review

dfac98d

Apply suggestions. Co-authored-by: Nate Wilson <[email protected]>

ksokolovskyi force-pushed the add-widget-state-property-example-and-tests branch from a0655e1 to dfac98d Compare September 22, 2024 07:14

bleroux reviewed Sep 23, 2024

View reviewed changes

Apply review suggestions.

581b7a4

ksokolovskyi requested a review from bleroux September 23, 2024 14:25

bleroux approved these changes Sep 23, 2024

View reviewed changes

Apply review suggestions.

2dda07b

ksokolovskyi requested a review from bleroux September 23, 2024 17:22

bleroux approved these changes Sep 23, 2024

View reviewed changes

nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 23, 2024

auto-submit bot merged commit 96ba3c5 into flutter:master Sep 24, 2024

engine-flutter-autoroll mentioned this pull request Sep 24, 2024

Roll Flutter from 4ca51a1de597 to 9fcbcd4357b6 (13 revisions) flutter/packages#7689

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

10bec5e

…flutter#155315)

ksokolovskyi mentioned this pull request Sep 24, 2024

WidgetStateProperty is missing example. #155313

Closed

engine-flutter-autoroll mentioned this pull request Sep 24, 2024

Roll Flutter from 4ca51a1de597 to dc277b77894b (14 revisions) flutter/packages#7690

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

d23ddc2

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 24, 2024

Roll Flutter from 4ca51a1de597 to ecf1ad94d41a (15 revisions) flutter/packages#7691

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

13dcf81

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 24, 2024

Roll Flutter from 4ca51a1de597 to cf5c362bbebb (16 revisions) flutter/packages#7692

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

b11c1b1

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 24, 2024

Roll Flutter from 4ca51a1de597 to f83b08495a2f (17 revisions) flutter/packages#7693

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 24, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

488788d

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to 4d1f086a649f (33 revisions) flutter/packages#7695

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

4748769

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to ce24dd6a76d2 (34 revisions) flutter/packages#7697

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

d03d06f

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to 048e38976159 (35 revisions) flutter/packages#7698

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

3bbb94d

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to f310625bc6df (37 revisions) flutter/packages#7700

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

d18b7a9

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to 1c9607fc9c98 (38 revisions) flutter/packages#7702

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

9e9464e

…flutter#155315)

engine-flutter-autoroll mentioned this pull request Sep 25, 2024

Roll Flutter from 4ca51a1de597 to 538e7424d018 (40 revisions) flutter/packages#7705

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 25, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

fe72961

…flutter#155315)

Piinks mentioned this pull request Sep 27, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

5f929ad

…flutter#155315)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024

96ba3c5 Add WidgetStateProperty example and tests for it. (flutter/…

bb18ed8

…flutter#155315)

		final TestGesture gesture =
		await tester.createGesture(kind: PointerDeviceKind.mouse);

Add WidgetStateProperty example and tests for it. #155315

Add WidgetStateProperty example and tests for it. #155315

Uh oh!

Conversation

ksokolovskyi commented Sep 17, 2024

Description

Pre-launch Checklist

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

ksokolovskyi commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nate-thegrate commented Sep 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ksokolovskyi commented Sep 18, 2024

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ksokolovskyi commented Sep 18, 2024

Uh oh!

bleroux left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ksokolovskyi commented Sep 23, 2024

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

bleroux Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

bleroux Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

ksokolovskyi commented Sep 23, 2024

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `WidgetStateProperty` example and tests for it. #155315

Add `WidgetStateProperty` example and tests for it. #155315

ksokolovskyi commented Sep 18, 2024 •

edited

Loading

nate-thegrate commented Sep 18, 2024 •

edited

Loading

bleroux left a comment •

edited

Loading