Skip to content

Fixed -> DropdownMenu throws exception when it is in any scrollable l… #140566

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
auto-submit merged 9 commits into from
Mar 7, 2024
Merged

Fixed -> DropdownMenu throws exception when it is in any scrollable l… #140566

auto-submit merged 9 commits into from
Mar 7, 2024

Conversation

@faisalansari0367
Copy link
Contributor

@faisalansari0367 faisalansari0367 commented Dec 23, 2023

Fixed -> DropdownMenu throws exception when it is in any scrollable list view and scrolls quickly #139871

@flutter-dashboard
Copy link

flutter-dashboard bot commented Dec 23, 2023

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact "@test-exemption-reviewer" in the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Dec 23, 2023
@faisalansari0367
Copy link
Contributor Author

faisalansari0367 commented Dec 23, 2023

I'd appreciate some assistance in writing the necessary tests to cover the changes introduced in this PR.

Could someone with expertise in testing or familiarity with this part of the codebase kindly review and contribute by creating the required tests? Your input will significantly enhance the reliability and robustness of the code.

Thank you in advance for your collaboration!

@navaronbracke
Copy link
Contributor

navaronbracke commented Dec 23, 2023 •
edited
Loading

You could write a widget test that does the following:

  • pump a ListView.builder with the WidgetTester; this widget contains a lot of Containers and a dropdown somewhere in the middle
  • start a fling gesture with a high enough velocity that scrolls the list fast enough to both create the Dropdown widget state and to make the widget unmount before the post frame callback ran
  • dispose of the gesture that you created, at the end of the test
  • add an expect(tester.takeException(), isNull); to verify that no exception happened

@faisalansari0367
Copy link
Contributor Author

faisalansari0367 commented Dec 24, 2023

You could write a widget test that does the following:

  • pump a ListView.builder with the WidgetTester; this widget contains a lot of Containers and a dropdown somewhere in the middle
  • start a fling gesture with a high enough velocity that scrolls the list fast enough to both create the Dropdown widget state and to make the widget unmount before the post frame callback ran
  • dispose of the gesture that you created, at the end of the test
  • add an expect(tester.takeException(), isNull); to verify that no exception happened

Thanks @navaronbracke

I have added the tests for the same.

goderbauer
goderbauer reviewed Jan 5, 2024
View reviewed changes
@HansMuller HansMuller requested a review from QuncCccccc January 5, 2024 23:00
QuncCccccc
QuncCccccc reviewed Feb 6, 2024
View reviewed changes
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for the contribution! This looks good:) Could you help rebase master to fix the conflicts?

@faisalansari0367
Copy link
Contributor Author

faisalansari0367 commented Mar 7, 2024

@QuncCccccc I have merged it with master, Please approve this PR.
Thanks

QuncCccccc
QuncCccccc approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM:) Thanks for the contribution!

MitchellGoodwin
MitchellGoodwin approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thank you for putting this together.

@QuncCccccc QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 7, 2024
@auto-submit auto-submit bot merged commit 874804e into flutter:master Mar 7, 2024
@HansMuller HansMuller mentioned this pull request Mar 7, 2024
Closed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 8, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 8, 2024
@engine-flutter-autoroll
874804e Fixed -> DropdownMenu throws exception when it is in any scro…
989baf0 
…llable l… (flutter/flutter#140566)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 8, 2024
@engine-flutter-autoroll
Roll Flutter from 471a828 to 7c89ec8 (15 revisions) (#6288)
a10b360 
flutter/flutter@471a828...7c89ec8

2024-03-08 [email protected] Remove `toString()` overrides in `dart:ui`/`package:flutter` in profile/release mode on wasm/vm targets (flutter/flutter#144763)
2024-03-08 [email protected] Roll Flutter Engine from 80cd7981043f to bbb1ed00af49 (3 revisions) (flutter/flutter#144813)
2024-03-08 [email protected] Update documentation of `AlertDialog`'s default `TextStyle` for Material 3 (flutter/flutter#144697)
2024-03-08 [email protected] Roll Flutter Engine from 175069397a40 to 80cd7981043f (5 revisions) (flutter/flutter#144804)
2024-03-07 [email protected] Enable asset transformation for `flutter run -d <browser>` and `flutter test` (flutter/flutter#144734)
2024-03-07 [email protected] Roll Flutter Engine from ec4f9af3d53b to 175069397a40 (2 revisions) (flutter/flutter#144799)
2024-03-07 [email protected] refactor: Remove `throwOnPluginPubspecError` flag for plugin validation (flutter/flutter#144214)
2024-03-07 [email protected] Roll Flutter Engine from 305333c50191 to ec4f9af3d53b (1 revision) (flutter/flutter#144797)
2024-03-07 [email protected] Roll Flutter Engine from c2863530ce39 to 305333c50191 (1 revision) (flutter/flutter#144793)
2024-03-07 [email protected] Fixed -> DropdownMenu throws exception when it is in any scrollable lâ�¦ (flutter/flutter#140566)
2024-03-07 [email protected] Roll Flutter Engine from 9e8ccaa3842e to c2863530ce39 (2 revisions) (flutter/flutter#144792)
2024-03-07 [email protected] Roll Flutter Engine from 12828950890a to 9e8ccaa3842e (1 revision) (flutter/flutter#144784)
2024-03-07 [email protected] Marks Mac_x64 module_test_ios to be flaky (flutter/flutter#144681)
2024-03-07 [email protected] [flutter_tools] add custom tool analysis to analyze.dart, lint Future.catchError (flutter/flutter#140122)
2024-03-07 [email protected] Roll Flutter Engine from f8c3b2db8cd1 to 12828950890a (1 revision) (flutter/flutter#144775)

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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
874804e Fixed -> DropdownMenu throws exception when it is in any scro…
caafe55 
…llable l… (flutter/flutter#140566)
@victorsanni victorsanni mentioned this pull request Oct 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@goderbauer goderbauer goderbauer left review comments

@QuncCccccc QuncCccccc QuncCccccc approved these changes

@MitchellGoodwin MitchellGoodwin MitchellGoodwin approved these changes

Assignees

No one assigned

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@faisalansari0367 @navaronbracke @goderbauer @QuncCccccc @MitchellGoodwin