Skip to content

Refactor chip class and move independent chips into separate classes#101507

Merged
fluttergithubbot merged 3 commits intoflutter:masterfrom
NevercodeHQ:chip_widgets_refactor
Apr 12, 2022
Merged

Refactor chip class and move independent chips into separate classes#101507
fluttergithubbot merged 3 commits intoflutter:masterfrom
NevercodeHQ:chip_widgets_refactor

Conversation

@TahaTesser
Copy link
Contributor

@TahaTesser TahaTesser commented Apr 7, 2022

Currently, independent chips like ActionChip, ChoiceChip, FilterChip, and InputChip are part of chip.dart and the new M3 migration will add two new chips with new names, chip.dart is already bloated.

fixes #101504

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.
  • 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 Apr 7, 2022
@TahaTesser TahaTesser self-assigned this Apr 7, 2022
@TahaTesser TahaTesser requested a review from HansMuller April 7, 2022 13:30
@TahaTesser TahaTesser removed their assignment Apr 7, 2022
@CaseyHillers CaseyHillers mentioned this pull request Apr 7, 2022
Closed
HansMuller
HansMuller approved these changes Apr 7, 2022
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

I realize that you're just moving classes into new files here, so my comments about being consistent wrt to public/private helper methods just applies to the pre-existing code.
a
This looks like a healthy refactor.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented Apr 8, 2022

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@HansMuller
Copy link
Contributor

HansMuller commented Apr 8, 2022 •
edited
Loading

@guidezpl - The "Google testing" failure is:

ERROR: mobile/flutter/tools/icon_font/bin/icon_map_generator.dart

46: final icon = Icon(entry); 2 positional argument(s) expected, but 1 found. #not_enough_positional_arguments

Presumably this has already been fixed?

@HansMuller
Copy link
Contributor

HansMuller commented Apr 8, 2022

@TahaTesser - I'm guessing that what needs to be done here is to rebase and push another commit. The Google testing failure doesn't appear to have anything to do with this PR.

@TahaTesser
Copy link
Contributor Author

TahaTesser commented Apr 8, 2022

@HansMuller

I made other private helper functions public since most of them are public even in other classes and add the comments to the material helper function :)

Yes, Google testing has been affecting many PRs lately, I will rebase now.

Some PRs just get stuck at Google testing such as
#101345
#101512

@TahaTesser TahaTesser force-pushed the chip_widgets_refactor branch from 1539362 to 4af28c4 Compare April 8, 2022 17:35
@guidezpl
Copy link
Member

guidezpl commented Apr 8, 2022

Yeah this was fixed internally, so all PRs need to rebase.

@TahaTesser TahaTesser force-pushed the chip_widgets_refactor branch from 4af28c4 to 0c766b1 Compare April 11, 2022 07:15
@TahaTesser TahaTesser force-pushed the chip_widgets_refactor branch from 0c766b1 to 9b7257a Compare April 12, 2022 06:07
@fluttergithubbot fluttergithubbot merged commit 0f2b1a3 into flutter:master Apr 12, 2022
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 12, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 12, 2022
@engine-flutter-autoroll
0f2b1a3 Refactor chip class and move independent chips into separate …
a340ec4 
…classes (flutter/flutter#101507)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Apr 12, 2022
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Apr 12, 2022
@engine-flutter-autoroll
0f2b1a3 Refactor chip class and move independent chips into separate …
b8f2cf1 
…classes (flutter/flutter#101507)
@TahaTesser TahaTesser deleted the chip_widgets_refactor branch April 12, 2022 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@HansMuller HansMuller HansMuller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Refactor chip.dart: Move independent chips into separate classes

4 participants

@TahaTesser @fluttergithubbot @HansMuller @guidezpl