Add findChildIndexCallback examples #133469

gildaswise · 2023-08-28T17:03:29Z

The documentation for using findChildIndexCallback recommends using indexOf, but that causes this line to throw in debug mode, and when using SliverList, it breaks the render.

This PR changes the usage to check if the index is not negative before using it, and changes to return null instead if the child wasn't able to be found.

There's the related issue #107123, but this doesn't actually fix it.

This PR has been updated to add the snippets that were used in the findChildIndexCallback comment as examples with proper tests, as well as updating the comment to reference the new examples.

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.

Piinks

Hi @gildaswise thanks for contributing! It looks like we should fix the issue with indexOf returning invalid indices, rather than changing our samples to reflect a workaround. What do you think?

gildaswise · 2023-08-30T19:23:54Z

Hey Kate, thanks for reviewing!

It looks like we should fix the issue with indexOf returning invalid indices

I didn't understand if there's something to be fixed. The findChildIndexCallback expects a nullable index as a return, and unless we change the processing to ignore negative indexes, I believe the correct usage should still be checking for the return of indexOf before returning in the callback, since it should support not finding the index.

gildaswise · 2023-09-01T23:47:34Z

I just noticed that the findChildIndexCallback says this:

This callback should take an input [Key], and it should return the index of the child element with that associated key, or null if not found.

So indeed the recommended callback in the documentation was wrong, it's not supposed to return -1 if not found, but null as I changed.

Piinks

Ah thank you for explaining. I see what you mean.

I think we should likely change these snippets to be dartpad examples. That way we can write a test for each to ensure this works as expected and does not regress in the future. Those examples live in examples/api, and are embedded using {@tool dartpad} instead of {@tool snippet}.

gildaswise · 2023-09-12T13:10:16Z

I think we should likely change these snippets to be dartpad examples.

I can try to do this in this PR then, will look into it once I have time this week. Thank you!

goderbauer · 2023-10-02T21:04:02Z

(triage): @gildaswise Do you still have plans to continue working on this PR in the near future?

gildaswise · 2023-10-02T21:21:04Z

@goderbauer Sorry about the delay, been a rough few weeks for me. I'll try to add the dartpad examples this week for sure, and if I can't, I'll ping you here

Piinks

Thanks so much for contributing this! Converting these to dartpads make them even more helpful as interactive examples, plus the tests you added ensure they aren't accidentally broken in the future. :)

gildaswise · 2023-10-11T14:43:13Z

Thanks so much for contributing this! Converting these to dartpads make them even more helpful as interactive examples, plus the tests you added ensure they aren't accidentally broken in the future. :)

Thank you for the review, Kate! I can't merge it myself but the CI passed now, please let me know if I need to do something else.

Piinks · 2023-10-11T16:03:01Z

Just waiting on a second review for this to land, I will reach out again to see if we can get one. :)

cbracken · 2023-10-17T22:23:16Z

examples/api/lib/widgets/scroll_view/list_view.1.dart

nit: add a period at the end of the comment and remove the blank line so it's snug against main.

Style guide ref: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use-correct-grammar

cbracken · 2023-10-17T22:23:35Z

examples/api/lib/widgets/page_view/page_view.1.dart

Nit: Add a period at the end of the comment.

Style guide ref: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#use-correct-grammar

examples/api/lib/widgets/page_view/page_view.1.dart

examples/api/lib/widgets/scroll_view/list_view.1.dart

Manual roll Flutter from 6eea6e2 to 189196d (44 revisions) Manual roll requested by [email protected] flutter/flutter@6eea6e2...189196d 2023-10-18 [email protected] Roll Flutter Engine from ab86c53c19cd to 6caee3236d37 (2 revisions) (flutter/flutter#136834) 2023-10-18 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Reland] Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136839) 2023-10-18 [email protected] Convert menus to use OverlayPortal (flutter/flutter#130534) 2023-10-18 [email protected] [Reland] Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136751) 2023-10-18 [email protected] Roll Flutter Engine from 0eff5d191856 to ab86c53c19cd (1 revision) (flutter/flutter#136832) 2023-10-18 [email protected] Fixes ability to call nextFocus() on a node to focus its descendant (flutter/flutter#136773) 2023-10-18 [email protected] Roll Flutter Engine from b778a07f8ae9 to 0eff5d191856 (6 revisions) (flutter/flutter#136829) 2023-10-18 [email protected] Implement GApplication:shutdown so a Flutter developer knows where to put code that should occur on application shutdown. (flutter/flutter#136780) 2023-10-18 [email protected] Roll Flutter Engine from 7f37c9b181af to b778a07f8ae9 (1 revision) (flutter/flutter#136818) 2023-10-18 [email protected] Fix `Slider` `onChanged` callback order & never calls `onChangeStart` on `SliderInteraction.slideOnly` allowed interaction (flutter/flutter#136720) 2023-10-18 [email protected] Roll Flutter Engine from 78026b4003fe to 7f37c9b181af (1 revision) (flutter/flutter#136817) 2023-10-18 [email protected] [Feat] Stroke color for Slider value indicator (flutter/flutter#135986) 2023-10-18 [email protected] Fixed : Empty Rows shown at last page in Paginated data table (flutter/flutter#132646) 2023-10-18 [email protected] Roll Flutter Engine from 46923fd39032 to 78026b4003fe (1 revision) (flutter/flutter#136814) 2023-10-18 [email protected] Roll Packages from d439062 to 14aa69e (1 revision) (flutter/flutter#136813) 2023-10-18 [email protected] Don't build native assets in `flutter build bundle` (flutter/flutter#136641) 2023-10-18 [email protected] Roll Flutter Engine from c9c9684e03a3 to 46923fd39032 (1 revision) (flutter/flutter#136795) 2023-10-18 [email protected] Roll Flutter Engine from 0c1c29271e8b to c9c9684e03a3 (1 revision) (flutter/flutter#136792) 2023-10-18 [email protected] Roll Flutter Engine from 1de09d13e708 to 0c1c29271e8b (3 revisions) (flutter/flutter#136789) 2023-10-18 [email protected] Roll Flutter Engine from 6fc36e61a99a to 1de09d13e708 (1 revision) (flutter/flutter#136785) 2023-10-18 [email protected] Roll Flutter Engine from 3f818efff3c5 to 6fc36e61a99a (1 revision) (flutter/flutter#136781) 2023-10-18 [email protected] Roll Flutter Engine from 2eef9b43cfb4 to 3f818efff3c5 (3 revisions) (flutter/flutter#136777) 2023-10-18 [email protected] Roll Flutter Engine from 5df7af34a718 to 2eef9b43cfb4 (4 revisions) (flutter/flutter#136774) 2023-10-18 [email protected] Flutter preview device (flutter/flutter#135639) 2023-10-18 [email protected] Add findChildIndexCallback examples (flutter/flutter#133469) 2023-10-18 [email protected] Adds API for performing semantics actions in tests (flutter/flutter#132598) 2023-10-18 [email protected] Roll Flutter Engine from e57b5bac4244 to 5df7af34a718 (2 revisions) (flutter/flutter#136772) 2023-10-17 [email protected] Roll Flutter Engine from f9f937e51080 to e57b5bac4244 (3 revisions) (flutter/flutter#136768) 2023-10-17 [email protected] clean up `--dart-define-from-file` option tests (flutter/flutter#135980) 2023-10-17 [email protected] GoldenFileComparators should dispose created Image objects. (flutter/flutter#136716) 2023-10-17 [email protected] make integration_test_driver_extended.dart support writeResponseData--(done) (flutter/flutter#128382) 2023-10-17 49699333+dependabot[bot]@users.noreply.github.com Bump actions/checkout from 3.6.0 to 4.1.1 (flutter/flutter#136762) 2023-10-17 [email protected] Roll Flutter Engine from 289f29b1ad00 to f9f937e51080 (1 revision) (flutter/flutter#136755) 2023-10-17 [email protected] Roll Flutter Engine from 659e68a097b5 to 289f29b1ad00 (1 revision) (flutter/flutter#136752) 2023-10-17 [email protected] Support --web-header option for flutter run (flutter/flutter#136297) 2023-10-17 [email protected] Add Android 14 physical devices to firebase tests (flutter/flutter#136736) 2023-10-17 [email protected] Roll Flutter Engine from 62a90a91cee3 to 659e68a097b5 (3 revisions) (flutter/flutter#136746) 2023-10-17 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Skip injecting Bonjour settings when port publication is disabled" (flutter/flutter#136750) 2023-10-17 [email protected] Revert "[SingleChildScrollView] Correct the offset pixels if it is out of range during layout" (flutter/flutter#136744) 2023-10-17 [email protected] Skip injecting Bonjour settings when port publication is disabled (flutter/flutter#136562) 2023-10-17 [email protected] Roll Flutter Engine from 3ecbe924a598 to 62a90a91cee3 (2 revisions) (flutter/flutter#136739) 2023-10-17 [email protected] Roll Flutter Engine from 0a4d8b99a95b to 3ecbe924a598 (3 revisions) (flutter/flutter#136732) 2023-10-17 [email protected] Reenable NexusLowRes API 29 (flutter/flutter#136686) 2023-10-17 [email protected] Reenable the nexus 6p tests (flutter/flutter#136689) ...

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. labels Aug 28, 2023

Piinks added the d: api docs Issues with https://api.flutter.dev/ label Aug 30, 2023

Piinks reviewed Aug 30, 2023

View reviewed changes

gildaswise requested a review from Piinks September 1, 2023 23:47

Piinks reviewed Sep 11, 2023

View reviewed changes

github-actions bot added the d: examples Sample code and demos label Oct 2, 2023

gildaswise changed the title ~~Update findChildIndexCallback comment~~ Add findChildIndexCallback examples Oct 2, 2023

gildaswise requested a review from Piinks October 2, 2023 22:35

Piinks approved these changes Oct 10, 2023

View reviewed changes

cbracken reviewed Oct 17, 2023

View reviewed changes

cbracken approved these changes Oct 17, 2023

View reviewed changes

Piinks reviewed Oct 17, 2023

View reviewed changes

examples/api/lib/widgets/page_view/page_view.1.dart Outdated Show resolved Hide resolved

examples/api/lib/widgets/scroll_view/list_view.1.dart Outdated Show resolved Hide resolved

gildaswise added 9 commits October 17, 2023 17:31

Update page_view.dart

576f7f2

Update scroll_view.dart

d7e5d46

Update scroll_view.dart

052ec71

Update page_view.dart

8824001

Update scroll_view.dart

070aa92

Create page_view.1.dart

6e29919

Create page_view.1_test.dart

249e68b

Update page_view.1.dart

5a34005

Update page_view.dart

3df8e06

gildaswise and others added 8 commits October 17, 2023 17:31

Update page_view.1_test.dart

4ce6c8e

Update list_view.1_test.dart

1a108ae

Update page_view.1.dart

a6f50c0

Update list_view.1.dart

37db26e

Update page_view.1_test.dart

416b4c6

Update list_view.1_test.dart

ff097ea

Update list_view.1_test.dart

daf4f6f

Apply suggestions from code review

32d3737

Piinks force-pushed the patch-1 branch from 5c9a0ed to 32d3737 Compare October 17, 2023 22:31

Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 17, 2023

auto-submit bot merged commit 492b6f7 into flutter:master Oct 18, 2023

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to 402a5455d987 (29 revisions) flutter/packages#5168

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

3850d9b

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to 8e4292b14407 (31 revisions) flutter/packages#5169

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

514a2bb

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to d195d90dc6ff (34 revisions) flutter/packages#5170

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

6920958

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to 4f959b97a795 (35 revisions) flutter/packages#5171

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

e17344c

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to fe9d9a9db3dd (37 revisions) flutter/packages#5172

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

3f5b675

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Roll Flutter from 6eea6e277e56 to f847346d9c8e (38 revisions) flutter/packages#5175

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

757b1a9

engine-flutter-autoroll mentioned this pull request Oct 18, 2023

Manual roll Flutter from 6eea6e277e56 to 189196d104e2 (44 revisions) flutter/packages#5178

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Oct 18, 2023

492b6f7 Add findChildIndexCallback examples (flutter/flutter#133469)

c6e7c79

HansMuller mentioned this pull request Oct 20, 2023

Flutter Framework Team: Notable Commits #121415

Closed

gildaswise deleted the patch-1 branch October 23, 2023 14:01

Add findChildIndexCallback examples #133469

Add findChildIndexCallback examples #133469

Uh oh!

Conversation

gildaswise commented Aug 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

gildaswise commented Aug 30, 2023

Uh oh!

gildaswise commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

gildaswise commented Sep 12, 2023

Uh oh!

goderbauer commented Oct 2, 2023

Uh oh!

gildaswise commented Oct 2, 2023

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

gildaswise commented Oct 11, 2023

Uh oh!

Piinks commented Oct 11, 2023

Uh oh!

cbracken Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

cbracken Oct 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gildaswise commented Aug 28, 2023 •

edited

Loading

gildaswise commented Sep 1, 2023 •

edited

Loading