Fix `GlowingOverscrollIndicator` examples by miechoo · Pull Request #155203 · flutter/flutter

miechoo · 2024-09-14T15:22:45Z

This pull request adds tests for the example files shown in the GlowingOverscrollIndicator Flutter API reference documentation.

nate-thegrate

I went ahead and pushed the example updates to this branch.

We should be good to add tests that look for the GlowingOverscrollIndicator now!

miechoo · 2024-09-20T09:50:43Z

Please let me know if these tests are sufficient. I encountered some unexpected behaviors during the test, so I decided not to test behaviors for now.

Different occurrences

I tried:

testWidgets('raw occurrences', (WidgetTester tester) async {
    await tester.pumpWidget(const example.GlowingOverscrollIndicatorExampleApp());
    final Finder goiFinder = find.byType(GlowingOverscrollIndicator);
    print(goiFinder);
});

and when testing glowing_overscroll_indicator.0.dart the output returns one widget which is expected, but when testing glowing_overscroll_indicator.1.dart the output returns two widgets which is a bit confusing to me. In the second case, the widget tree structure is:

- GlowingOverscrollIdicatorExampleApp
  - MaterialApp
    - Scaffold
      - GlowingOverscrollIdicatorExample
        - NestedScrollView
          - GlowingOverscrollIdicator
            - SliverAppBar
              - Text
            - CustomScrollView
              - GlowingOverscrollIdicator
                - ...
                  - ...

First I thought it was a result of using NestedScrollView, but I guess it's not.

Dragging and calculating the position

I tried to test both examples using await tester.drag() and tester.getBottomLeft(goiFinder).dy to check behaviors and calculate positions, but in both cases (glowing_overscroll_indicator.0.dart and glowing_overscroll_indicator.1.dart) I got inconsistent and incomprehensible results.

So, I hope that the visibility tests will be sufficient.

nate-thegrate · 2024-09-20T17:19:06Z

in both cases (glowing_overscroll_indicator.0.dart and glowing_overscroll_indicator.1.dart) I got inconsistent and incomprehensible results.

After looking it over again, I think that was my fault 😅
I copy-pasted the glowing_overscroll_indicator.0.dart changes into glowing_overscroll_indicator.1.dart, not realizing that it was adding an extra AppBar that shouldn't have been there.

I'll go ahead and push the fix (and restore the tests you added previously); if everything passes then I think we'll be good to merge!

TahaTesser

Thanks for contributing!

nate-thegrate

@TahaTesser thanks very much for the review! Since I contributed the example updates, I'll respond to those; I'll leave test updates to @miechoo :)

miechoo · 2024-09-29T19:39:59Z

Hi guys. Should we do something more with this PR?

nate-thegrate · 2024-09-29T19:42:57Z

Generally we give code reviewers 2 weeks before pinging them, since they often have busy lives with many responsibilities. 🙂

(more info here)

bleroux

LGTM

TahaTesser

LGMT with nit

…oll_indicator.0_test.dart Co-authored-by: Bruno Leroux <[email protected]>

Co-authored-by: Taha Tesser <[email protected]>

auto-submit · 2024-10-30T20:04:08Z

auto label is removed for flutter/flutter/155203, due to - The status or check suite Windows tool_integration_tests_2_9 has failed. Please fix the issues identified (or deflake) before re-applying this label.

…5203)

Manual roll requested by [email protected] flutter/flutter@fe71cad...0fe6153 2024-10-31 [email protected] Roll Flutter Engine from c40b0b602822 to f2154ef3e31c (8 revisions) (flutter/flutter#157926) 2024-10-31 [email protected] Hides added routes before top-most route finishes pushing (flutter/flutter#156104) 2024-10-31 [email protected] Fix cursor on hover expand/collapse icon (#155207) (flutter/flutter#155209) 2024-10-31 [email protected] Add test for `media_query_data.system_gesture_insets.0.dart` (flutter/flutter#157854) 2024-10-31 [email protected] Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. (flutter/flutter#157879) 2024-10-31 [email protected] Roll Flutter Engine from 090c33aeae83 to c40b0b602822 (1 revision) (flutter/flutter#157896) 2024-10-31 [email protected] Roll Flutter Engine from 9295eeb6d3ce to 090c33aeae83 (4 revisions) (flutter/flutter#157893) 2024-10-30 [email protected] Roll Flutter Engine from 2bd854e23b61 to 9295eeb6d3ce (5 revisions) (flutter/flutter#157882) 2024-10-30 [email protected] Adds a new helpful tool exit message for SocketExceptions thrown during mdns discovery (flutter/flutter#157638) 2024-10-30 [email protected] Fix `GlowingOverscrollIndicator` examples (flutter/flutter#155203) 2024-10-30 [email protected] Roll Flutter Engine from 906a7ad88052 to 2bd854e23b61 (1 revision) (flutter/flutter#157878) 2024-10-30 [email protected] Upgrade templates to AGP 8.7/Gradle 8.10.2 (flutter/flutter#157872) 2024-10-30 [email protected] Roll Flutter Engine from 57ed5d338e7e to 906a7ad88052 (13 revisions) (flutter/flutter#157875) 2024-10-30 [email protected] Update Material 3 `LinearProgressIndicator` for new visual style (flutter/flutter#154817) 2024-10-30 [email protected] Roll Flutter Engine from 999797a2f690 to 57ed5d338e7e (5 revisions) (flutter/flutter#157833) 2024-10-30 [email protected] Add hidden `--no-implicit-pubspec-resolution` flag for one stable release. (flutter/flutter#157635) 2024-10-30 [email protected] Roll Packages from 028027e to 7cc1caa (5 revisions) (flutter/flutter#157864) 2024-10-30 [email protected] Mention partial PRs in the contributing docs (flutter/flutter#157863) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…5203)

) Manual roll requested by [email protected] flutter/flutter@fe71cad...0fe6153 2024-10-31 [email protected] Roll Flutter Engine from c40b0b602822 to f2154ef3e31c (8 revisions) (flutter/flutter#157926) 2024-10-31 [email protected] Hides added routes before top-most route finishes pushing (flutter/flutter#156104) 2024-10-31 [email protected] Fix cursor on hover expand/collapse icon (#155207) (flutter/flutter#155209) 2024-10-31 [email protected] Add test for `media_query_data.system_gesture_insets.0.dart` (flutter/flutter#157854) 2024-10-31 [email protected] Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. (flutter/flutter#157879) 2024-10-31 [email protected] Roll Flutter Engine from 090c33aeae83 to c40b0b602822 (1 revision) (flutter/flutter#157896) 2024-10-31 [email protected] Roll Flutter Engine from 9295eeb6d3ce to 090c33aeae83 (4 revisions) (flutter/flutter#157893) 2024-10-30 [email protected] Roll Flutter Engine from 2bd854e23b61 to 9295eeb6d3ce (5 revisions) (flutter/flutter#157882) 2024-10-30 [email protected] Adds a new helpful tool exit message for SocketExceptions thrown during mdns discovery (flutter/flutter#157638) 2024-10-30 [email protected] Fix `GlowingOverscrollIndicator` examples (flutter/flutter#155203) 2024-10-30 [email protected] Roll Flutter Engine from 906a7ad88052 to 2bd854e23b61 (1 revision) (flutter/flutter#157878) 2024-10-30 [email protected] Upgrade templates to AGP 8.7/Gradle 8.10.2 (flutter/flutter#157872) 2024-10-30 [email protected] Roll Flutter Engine from 57ed5d338e7e to 906a7ad88052 (13 revisions) (flutter/flutter#157875) 2024-10-30 [email protected] Update Material 3 `LinearProgressIndicator` for new visual style (flutter/flutter#154817) 2024-10-30 [email protected] Roll Flutter Engine from 999797a2f690 to 57ed5d338e7e (5 revisions) (flutter/flutter#157833) 2024-10-30 [email protected] Add hidden `--no-implicit-pubspec-resolution` flag for one stable release. (flutter/flutter#157635) 2024-10-30 [email protected] Roll Packages from 028027e to 7cc1caa (5 revisions) (flutter/flutter#157864) 2024-10-30 [email protected] Mention partial PRs in the contributing docs (flutter/flutter#157863) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

) Manual roll requested by [email protected] flutter/flutter@fe71cad...0fe6153 2024-10-31 [email protected] Roll Flutter Engine from c40b0b602822 to f2154ef3e31c (8 revisions) (flutter/flutter#157926) 2024-10-31 [email protected] Hides added routes before top-most route finishes pushing (flutter/flutter#156104) 2024-10-31 [email protected] Fix cursor on hover expand/collapse icon (#155207) (flutter/flutter#155209) 2024-10-31 [email protected] Add test for `media_query_data.system_gesture_insets.0.dart` (flutter/flutter#157854) 2024-10-31 [email protected] Add and plumb `useImplicitPubspecResolution` across `flutter_tools`. (flutter/flutter#157879) 2024-10-31 [email protected] Roll Flutter Engine from 090c33aeae83 to c40b0b602822 (1 revision) (flutter/flutter#157896) 2024-10-31 [email protected] Roll Flutter Engine from 9295eeb6d3ce to 090c33aeae83 (4 revisions) (flutter/flutter#157893) 2024-10-30 [email protected] Roll Flutter Engine from 2bd854e23b61 to 9295eeb6d3ce (5 revisions) (flutter/flutter#157882) 2024-10-30 [email protected] Adds a new helpful tool exit message for SocketExceptions thrown during mdns discovery (flutter/flutter#157638) 2024-10-30 [email protected] Fix `GlowingOverscrollIndicator` examples (flutter/flutter#155203) 2024-10-30 [email protected] Roll Flutter Engine from 906a7ad88052 to 2bd854e23b61 (1 revision) (flutter/flutter#157878) 2024-10-30 [email protected] Upgrade templates to AGP 8.7/Gradle 8.10.2 (flutter/flutter#157872) 2024-10-30 [email protected] Roll Flutter Engine from 57ed5d338e7e to 906a7ad88052 (13 revisions) (flutter/flutter#157875) 2024-10-30 [email protected] Update Material 3 `LinearProgressIndicator` for new visual style (flutter/flutter#154817) 2024-10-30 [email protected] Roll Flutter Engine from 999797a2f690 to 57ed5d338e7e (5 revisions) (flutter/flutter#157833) 2024-10-30 [email protected] Add hidden `--no-implicit-pubspec-resolution` flag for one stable release. (flutter/flutter#157635) 2024-10-30 [email protected] Roll Packages from dda8fe3 to 2a42664 (5 revisions) (flutter/flutter#157864) 2024-10-30 [email protected] Mention partial PRs in the contributing docs (flutter/flutter#157863) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. f: scrolling Viewports, list views, slivers, etc. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Sep 14, 2024

miechoo changed the title ~~130459 overscroll indicator~~ Missing tests of GlowingOverscrollIndicator Sep 14, 2024

nate-thegrate mentioned this pull request Sep 14, 2024

Fix the GlowingOverscrollIndicator examples #155208

Closed

This comment was marked as outdated.

Sign in to view

nate-thegrate marked this pull request as draft September 14, 2024 20:10

nate-thegrate self-requested a review September 17, 2024 16:29

nate-thegrate marked this pull request as ready for review September 17, 2024 16:32

nate-thegrate reviewed Sep 17, 2024

View reviewed changes

nate-thegrate changed the title ~~Missing tests of GlowingOverscrollIndicator~~ Fix GlowingOverscrollIndicator examples Sep 17, 2024

nate-thegrate approved these changes Sep 20, 2024

View reviewed changes

nate-thegrate requested review from TahaTesser and bleroux September 20, 2024 19:18

TahaTesser suggested changes Sep 23, 2024

View reviewed changes

nate-thegrate removed the request for review from bleroux September 23, 2024 15:12

nate-thegrate reviewed Sep 24, 2024

View reviewed changes

miechoo requested a review from TahaTesser September 25, 2024 18:09

bleroux reviewed Sep 30, 2024

View reviewed changes

Piinks force-pushed the 130459_overscroll_indicator branch from e97d01d to 460bcf5 Compare October 22, 2024 22:08

Piinks requested a review from bleroux October 22, 2024 22:08

bleroux approved these changes Oct 23, 2024

View reviewed changes

TahaTesser approved these changes Oct 30, 2024

View reviewed changes

Comment thread examples/api/lib/widgets/overscroll_indicator/glowing_overscroll_indicator.0.dart Outdated

Comment thread examples/api/lib/widgets/overscroll_indicator/glowing_overscroll_indicator.1.dart Outdated

miechoo and others added 2 commits October 30, 2024 13:22

test of OverscrollIndicatorNotification

0b743df

show the GlowingOverscrollIndicator

09e6c1c

miechoo and others added 7 commits October 30, 2024 13:22

adjustments following 155208

d317cdb

fix example, re-add tests

48112df

changes per TahaTesser's feedback

81ced5f

adjustments following 155208, comment review

467abd2

add comments per Bruno's feedback

0865497

Update examples/api/test/widgets/overscroll_indicator/glowing_overscr…

817849d

…oll_indicator.0_test.dart Co-authored-by: Bruno Leroux <[email protected]>

Apply suggestions from code review

5a5e70b

Co-authored-by: Taha Tesser <[email protected]>

nate-thegrate force-pushed the 130459_overscroll_indicator branch from 9d01435 to 5a5e70b Compare October 30, 2024 19:23

nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2024

auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2024

nate-thegrate added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 30, 2024

auto-submit Bot merged commit 2d2ebbe into flutter:master Oct 30, 2024

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to 16622e67a051 (15 revisions) flutter/packages#7971

Closed

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

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

4d214ee

…5203)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to b01558c1cd70 (16 revisions) flutter/packages#7972

Closed

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

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

6936d38

…5203)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Roll Flutter from fe71cadf1959 to 0fe615343bd9 (18 revisions) flutter/packages#7973

Closed

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

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

96524d6

…5203)

engine-flutter-autoroll mentioned this pull request Oct 31, 2024

Manual roll Flutter from fe71cadf1959 to 0fe615343bd9 (18 revisions) flutter/packages#7989

Merged

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

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

322095b

…5203)

Piinks mentioned this pull request Nov 8, 2024

Flutter Framework Team: Notable Commits #121415

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

ba3b0dd

…5203)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

b585b06

…5203)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

ce6de9a

…5203)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

6ac1870

…5203)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025

2d2ebbe Fix GlowingOverscrollIndicator examples (flutter/flutter#15…

f781fef

…5203)

Conversation

miechoo commented Sep 14, 2024 • edited by nate-thegrate Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

miechoo commented Sep 20, 2024

Different occurrences

Dragging and calculating the position

Uh oh!

nate-thegrate commented Sep 20, 2024

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

miechoo commented Sep 29, 2024

Uh oh!

nate-thegrate commented Sep 29, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bleroux left a comment

Choose a reason for hiding this comment

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

auto-submit Bot commented Oct 30, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

miechoo commented Sep 14, 2024 •

edited by nate-thegrate

Loading