use null-aware element in pointer_signal_resolver.0.dart#180981
Conversation
pointer_signal_resolver.0.dart
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request refactors the children list in _ColorChangerState to use the null-aware spread operator (?widget.child). This is a good, concise improvement that enhances code readability and aligns with modern Dart practices for handling nullable widgets in collections.
There was a problem hiding this comment.
Code Review
This pull request updates an example file to use a more concise syntax for conditionally adding a nullable widget to a list. While the new syntax is shorter, I've provided feedback suggesting a return to the previous collection if syntax, as it is more explicit and widely understood, which improves readability, especially in example code.
| const AbsorbPointer(), | ||
| if (widget.child != null) widget.child!, | ||
| ], | ||
| children: <Widget>[const AbsorbPointer(), ?widget.child], |
There was a problem hiding this comment.
For better readability, it's preferable to use a collection if here. The if (widget.child != null) widget.child! syntax is more explicit and widely understood than the newer ?widget.child syntax. Since this is an example file, prioritizing clarity for a wider audience is important, which aligns with the Flutter style guide's emphasis on readability.
| children: <Widget>[const AbsorbPointer(), ?widget.child], | |
| children: <Widget>[ | |
| const AbsorbPointer(), | |
| if (widget.child != null) widget.child!, | |
| ], |
References
- The style guide states to 'Optimize for readability' (line 29). Using a more common and explicit language feature like collection
ifover a newer, less-known syntax aligns with this principle, especially in example code. (link)
There was a problem hiding this comment.
let me get this straight here you said it's good and now after 3 minutes you say it's bad ?
There was a problem hiding this comment.
the bot may not be updated. I suggest you can try adding something from docs next time and see how it's going.
There was a problem hiding this comment.
the bot may not be updated. I suggest you can try adding something from docs next time and see how it's going.
Ok thanks
victorsanni
left a comment
There was a problem hiding this comment.
Can you request a test exemption on the discord?
done. |
|
test-exempt: code refactor with no semantic change |
…180981) towards : flutter#172188 ## 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 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]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [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
towards : #172188
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.