Implement CupertinoListSection and CupertinoListTile
#78732
Conversation
flutter-dashboard
bot
added
f: cupertino
|
Gold has detected about 5 untriaged digest(s) on patchset 2. |
|
Gold has detected about 5 untriaged digest(s) on patchset 3. |
|
Gold has detected about 5 untriaged digest(s) on patchset 4. |
campovski
changed the title
CupertinoListSection and CupertinoListTileCupertinoListSection and CupertinoListTile
campovski
force-pushed
the
SLFLF-3
branch
3 times, most recently
from
f9f2135
to
06b8193
Compare
|
hi @campovski do you happen to have the source of demo screens from above available? It looks great and thank you for the work on it. |
|
@campovski thanks a lot for getting back to me and your time building the examples. I copies your code to latest flutter and it didn't give any error. Will try out the examples of usage. Thanks again for sharing your work. Cheers. |
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
|
Gold has detected about 57 new digest(s) on patchset 16. |
| /// The [onTap] function is called when a user taps on [CupertinoListTile]. If | ||
| /// left `null`, the [CupertinoListTile] will not show any visual information | ||
| /// when tapped. | ||
| final Future<void> Function()? onTap; |
There was a problem hiding this comment.
question: Why is this declared as final Future<void> Function()? onTap; rather than final void Function()? onTap; (or, in an equivalent form, final VoidCallback? onTap;)?
There was a problem hiding this comment.
You are right. I think I have had an usecase where this needed to be a Future<void> but it makes sense to turn this into a VoidCallback.
There was a problem hiding this comment.
This should not be a VoidCallback for the following reason: pressing a "list tile" on iOS that navigates to a new route, highlights that list tile. Once the route is poped, the list tile is still highlighted in remains to be so, until route is completely popped. If this function completes instantly, given current implementation (simple yet effective) the list tile become not highlighted instantly.
It would make sense to turn this into a FutureOr<void> function, however AFAIR this is against Flutter coding standards. I will however still convert this to a FutureOr<void> Function()? onTap.
|
@campovski At a high level this looks great. It probably needs some work e.g. resolving analysis issues and some code review comments and such, but before I get too into the weeds I wanted to ask you what the best way to help you is. Are you still interested in landing this? (I'm painfully aware that it has taken us months to get to it and I'm really sorry about that.) |
|
@Hixie Yes, I would like to land this, however unfortunately I haven't got time lately to work on Flutter in general. What I am a bit worried about this PR is organizing it into meaningful commits. I did my best to do so when I actively developed this feature but now it became a bit of a mess with last few commits. Nonetheless, I promise to resolve the analysis issues and comments tomorrow. Looking forward to it! |
|
Please don't feel like I'm rushing you, I'm obviously in no position to do so since we've been so slow here. Your contributions are massively welcome regardless. :-) |
|
@Hixie just wanted to let you know that this PR is now ready. The analysis errors have been fixed and CR comments either applied or (I believe justiable) answered. Please, explicitly say, if it is required, that this PR be organised into meaningful commits. |
|
No, no need to worry about the commits. It all gets squashed into one when it lands anyway. The only reason to worry about the commits is if doing so would make it notably easier to review (e.g. if one commit is all automated fixes created by a regular expression, or something, vs hand-written code). |
|
Ok, that was my thought as well but since this PR affects just a few files, it is probably redundant to organize commits, I agree. |
|
Which flutter stable version will include these new widgets? |
|
This merged in commit 0c40945 and right now is only available in the master channel. You can follow along on that commit as tags get added to it https://github.com/flutter/flutter/wiki/Where's-my-Commit%3F#finding-the-versions-that-contain-framework-commit-x |
See flutter#78732 (comment) for more details.
This pull request adds two important widgets that were missing, a
CupertinoListSection(a close relative of Material DesignListView) and aCupertinoListTile(a iOS-style twin of Material DesignListTile).Fixes #19228
Repo flutter/tests left intact.
Pre-launch Checklist
///).Pull Request steps
CupertinoListSectionandCupertinoListTile.CupertinoFormSectionandCupertinoFormRow.CupertinoListSectionandCupertinoListTile. These tests should be similar to the ones already present forCupertinoFormSectionandCupertinoFormRow.