Skip to content

Conversation

@ValentinVignal
Copy link
Contributor

Contributes to #130459

It adds a test for

  • examples/api/test/material/material_state/material_state_property.0_test.dart

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Oct 14, 2024
Copy link
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution !
See my comment about the finder vs helper.

const example.MaterialStatePropertyExampleApp(),
);

expect(findForegroundColor(Colors.red), findsOne);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a finder here feels somewhat unintuitive to me.
Maybe a helper function such as getButtonForegroundColor and an expect such as expect(getButtonForegroundColor(), Colors.red) would be easier to understand?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

);

testWidgets(
'The foreground color of the TextButton should blue when hovered',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'The foreground color of the TextButton should blue when hovered',
'The foreground color of the TextButton should be blue when hovered',

Copy link
Contributor

@bleroux bleroux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
You should propably rebase to fix the failing CI check.

@ValentinVignal ValentinVignal force-pushed the flutter/api/example/add-test-for-material-state-property branch from 1670c7f to eb9c756 Compare October 15, 2024 10:37
@ValentinVignal ValentinVignal added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 15, 2024
@auto-submit auto-submit bot merged commit 5d06db6 into flutter:master Oct 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 16, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 17, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants