[Material] modernize legacy switch statements to expressions across multiple files#181427
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging. If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group. |
There was a problem hiding this comment.
Code Review
This pull request modernizes several switch statements to use Dart 3's switch expressions across various Material library files. The changes in app_bar.dart, bottom_sheet.dart, dialog.dart, and drawer.dart are correct and improve code conciseness. I've added one suggestion for further simplification in drawer.dart. Overall, this is a great contribution to code modernization.
3afa6ee to
6a6c095
Compare
| if (theme.platform == TargetPlatform.iOS || theme.platform == TargetPlatform.macOS) { | ||
| return actions == null || actions!.length < 2; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
I think the code before the PR is better since it checks enum exhaustiveness.
| case TargetPlatform.windows: | ||
| platformHasBackButton = false; | ||
| } | ||
| final platformHasBackButton = defaultTargetPlatform == TargetPlatform.android; |
There was a problem hiding this comment.
Same as above: switch is better for enum for exhaustiveness check and we should not throw it away. Although you can change it to a switch expression, which can be more "modern".
There was a problem hiding this comment.
i understand you and agree with you. would you like me to make the necessary corrections and commit again?
|
Hi! @Enderjua Are you able to address the comments above so we can work together to get this landed? This is a great addition and I'm looking forward to it! |
|
Hello @dkwingsmt thanks so much for the ping and the kind words. I started my university semester and in the shuffle of settling in this PR unfortunately slipped my mind. Sincere apologies for the silence. Pushing the updates shortly. |
c5587d8 to
cd78794
Compare
cd78794 to
9c5c9bb
Compare
dkwingsmt
left a comment
There was a problem hiding this comment.
LGTM. Great change, thank you!
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…s across multiple files (flutter/flutter#181427)
…ultiple files (flutter#181427) This PR modernizes legacy switch statements across several Material library files by converting them to Dart 3 switch expressions. While exploring `dialog.dart` and `drawer.dart`, I noticed a recurring pattern of using switch statements for platform-based variable assignments (like semantic labels and boolean flags). I've expanded the cleanup to include `app_bar.dart` and `bottom_sheet.dart` to ensure consistency across the -> All relevant tests passed (370+ tests total across ` dialog_test.dart`, `drawer_test.dart`, `app_bar_test.dart`, and `bottom_sheet_test.dart`). -> Verified with `flutter analyze` and `dart format` ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. (I will sign it once the bot prompts me) - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This PR modernizes legacy switch statements across several Material library files by converting them to Dart 3 switch expressions.
While exploring
dialog.dartanddrawer.dart, I noticed a recurring pattern of using switch statements for platform-based variable assignments (like semantic labels and boolean flags). I've expanded the cleanup to includeapp_bar.dartandbottom_sheet.dartto ensure consistency across the-> All relevant tests passed (370+ tests total across
dialog_test.dart,drawer_test.dart,app_bar_test.dart, andbottom_sheet_test.dart).-> Verified with
flutter analyzeanddart formatPre-launch Checklist
///).