Skip to content

Conversation

@cedvdb
Copy link
Contributor

@cedvdb cedvdb commented Jun 8, 2022

This PR removes the key on _IconButtonM3 as it currently duplicates the parent key. As stated in that issue I'm under the impression that keys should not be reused, since this is not the first time I see this in the flutter repo maybe they are ? In any case, I just removed the duplication in the child widget.

Fixes #105554

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.
  • [n/a] 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.
  • All existing and new tests are passing.

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

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jun 8, 2022
@cedvdb cedvdb force-pushed the fix_105554_duplicated_key branch from 34e811c to 9b1a0bc Compare June 8, 2022 16:38
@cedvdb cedvdb force-pushed the fix_105554_duplicated_key branch from 9b1a0bc to aa17460 Compare June 8, 2022 16:42
Copy link
Contributor

@darrenaustin darrenaustin 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. This looks reasonable to me. I don't think we are using the key on the internal _IconButtonM3, so I don't think this will cause in issues.

LGTM

@darrenaustin darrenaustin requested a review from QuncCccccc June 9, 2022 07:36
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks a lot for the fix!

@fluttergithubbot fluttergithubbot merged commit feec13a into flutter:master Jun 10, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 10, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 11, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 12, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 13, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 14, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

_IconButtonM3 reusing key

5 participants