Repository navigation
Conversation
github-actions
Bot
added
p: material_ui
triage-design
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds two widget tests to app_bar_sliver_test.dart to verify showOnScreen functionality with a pinned SliverAppBar using a sliver list and individual slivers. The review feedback recommends removing the k prefix from local constants to adhere to Dart style guidelines and replacing hardcoded pump delays with pumpAndSettle for more robust animation waiting.
|
Addressed both suggestions in c4cbb73: dropped the |
The widgets test 'showOnScreen works with pinned app bar and sliver list' in flutter/flutter was switched from SliverAppBar to a SliverPersistentHeader delegate in flutter/flutter#186611, which left the SliverAppBar path without coverage for SemanticsAction.showOnScreen. Add that coverage here, as the original TODO pointed at material_ui and Material is frozen in flutter/flutter. Fixes flutter/flutter#189117
flutter/flutter#186611 removed SliverAppBar from two showOnScreen tests, but only the sliver-list one was tracked by a TODO. Restore the individual-slivers case as well, which exercises FlexibleSpaceBar collapse behaviour that the SliverPersistentHeader stand-in does not.
kItemHeight was inherited from the upstream test but held 100.0 while the items were 72.0 tall; it was only ever used to compute the initial scroll offset. Make kItemHeight the actual item height and give the offset its own constant. No behaviour change: items are still 72.0 and the initial offset is still 250.0.
Matches this file's existing conventions.
c4cbb73 to
6e0fa3f
Compare
elliette
left a comment
elliette
left a comment
There was a problem hiding this comment.
Thanks for adding this test coverage! Left a few suggestions
QuncCccccc
left a comment
QuncCccccc
left a comment
There was a problem hiding this comment.
Thank you for your contribution! Because of the volume of PRs we receive, we require that new contributors use our checklist to guide them through critical steps in creating a Flutter PR. This PR's description is missing that checklist, so it is being marked as a Draft.
Please edit the PR description to add the checklist, then ensure that you have completed all of the steps. Once you've done that, please mark the PR as ready for review.
If you need help, consider asking for advice on the #hackers-new channel on Discord.
3 participants
Reopen of #12341, now that material_ui is accepting contributions (per flutter/flutter#188444).
Restores
showOnScreensemantics coverage for a pinnedSliverAppBarthat was lost in flutter/flutter#186611, per flutter/flutter#189117. Companion framework-only PR: flutter/flutter#190410.No version bump or CHANGELOG entry: test-only change, documented exemption.