Skip to content
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

Improve DropdownMenu sample code for requestFocusOnTap on mobile platforms #134867

Merged
merged 1 commit into from Sep 18, 2023
Merged

Improve DropdownMenu sample code for requestFocusOnTap on mobile platforms #134867

merged 1 commit into from Sep 18, 2023

Conversation

huycozy
Copy link
Member

@huycozy huycozy commented Sep 16, 2023 •
edited

Description

This PR is to improve DropdownMenu sample code. By default, requestFocusOnTap is false on mobile platforms. When users run API sample code on mobile platforms, they can not edit the text field and think it is a bug. Although it is detailed at https://api.flutter.dev/flutter/material/DropdownMenu/requestFocusOnTap.html, users often do not pay attention to it.

Related issue

Fixes #127672

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@huycozy huycozy self-assigned this Sep 16, 2023
@flutter-dashboard
Copy link

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 Hixie or stuartmorgan on 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. d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos labels Sep 16, 2023
@QuncCccccc
Copy link
Contributor

This looks good. Could you help to add a test in examples/api/test/material/dropdown_menu/dropdown_menu.0_test.dart?

@github-actions github-actions bot added the f: focus Focus traversal, gaining or losing focus label Sep 16, 2023
@huycozy
Copy link
Member Author

huycozy commented Sep 16, 2023

@QuncCccccc Thanks for taking a look. I just added a new test at dropdown_menu_focused_on_field.0_test.dart instead of appending to dropdown_menu.0_test.dart. Please help to review it again.

@QuncCccccc
Copy link
Contributor

@QuncCccccc Thanks for taking a look. I just added a new test at dropdown_menu_focused_on_field.0_test.dart instead of appending to dropdown_menu.0_test.dart. Please help to review it again.

Just curious why do we want to create a new file for the same sample file🙂? From my observation, seems there is only on test file for a specific sample file, so maybe we can just follow this pattern:) But let me know if there's any other concerns.
Other than that, all look good! Thanks!

@huycozy
Copy link
Member Author

huycozy commented Sep 17, 2023

Ah, I thought it would look neater because I saw dropdown_menu_entry_label_widget.0_test.dart. But I was wrong, it was a test for another example. I moved the new test into dropdown_menu.0_test.dart. Hope it's ok now :)

@huycozy
Improve DropdownMenu sample code for requestFocusOnTap when using on-…
aacfe45 
…screen virtual keyboard on mobile platforms

Signed-off-by: Nguyen Huy <huy@nevercode.io>
QuncCccccc
QuncCccccc approved these changes Sep 18, 2023
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 improvement:)

@huycozy huycozy added the autosubmit Merge PR when tree becomes green via auto submit App label Sep 18, 2023
@auto-submit auto-submit bot merged commit c2b1e48 into flutter:master Sep 18, 2023
37 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2023
@engine-flutter-autoroll
c2b1e483d Improve DropdownMenu sample code for requestFocusOnTap on m…
fa68996 
…obile platforms (flutter/flutter#134867)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Sep 18, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 18, 2023
@engine-flutter-autoroll
c2b1e483d Improve DropdownMenu sample code for requestFocusOnTap on m…
78ef6c1 
…obile platforms (flutter/flutter#134867)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Sep 18, 2023
@engine-flutter-autoroll
Roll Flutter from 1b18b1327a59 to b7d0e8c9ed70 (11 revisions) (#4944)
201e5c8 
flutter/flutter@1b18b13...b7d0e8c

2023-09-18 engine-flutter-autoroll@skia.org Roll Packages from bc8c2f2 to d4e2454 (6 revisions) (flutter/flutter#134945)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4107ee0dc8b1 to be7a039c5451 (1 revision) (flutter/flutter#134937)
2023-09-18 dacoharkes@google.com Native assets support for Linux (flutter/flutter#134031)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 77965cabbaf9 to 4107ee0dc8b1 (1 revision) (flutter/flutter#134927)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 75e5ba3c4671 to 77965cabbaf9 (1 revision) (flutter/flutter#134925)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 573e44b91887 to 75e5ba3c4671 (1 revision) (flutter/flutter#134923)
2023-09-18 104349824+huycozy@users.noreply.github.com Improve DropdownMenu sample code for requestFocusOnTap on mobile platforms (flutter/flutter#134867)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 51a402860f5b to 573e44b91887 (1 revision) (flutter/flutter#134920)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from f53681fb2220 to 51a402860f5b (1 revision) (flutter/flutter#134918)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9f3e46a21e51 to f53681fb2220 (1 revision) (flutter/flutter#134916)
2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 57a4ff3c7ff8 to 9f3e46a21e51 (1 revision) (flutter/flutter#134909)

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 dit@google.com,rmistry@google.com,stuartmorgan@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@HansMuller HansMuller mentioned this pull request Sep 28, 2023
Open
Mairramer pushed a commit to Mairramer/flutter that referenced this pull request Oct 10, 2023
@huycozy @Mairramer
Improve DropdownMenu sample code for requestFocusOnTap on mobile plat…
0a576f8 
…forms (flutter#134867)

### Description

This PR is to improve `DropdownMenu` sample code. By default, `requestFocusOnTap` is false on mobile platforms. When users run API sample code on mobile platforms, they can not edit the text field and think it is a bug. Although it is detailed at https://api.flutter.dev/flutter/material/DropdownMenu/requestFocusOnTap.html, users often do not pay attention to it.

### Related issue

Fixes flutter#127672
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 15, 2023
@engine-flutter-autoroll
c2b1e483d Improve DropdownMenu sample code for requestFocusOnTap on m…
1586cfd 
…obile platforms (flutter/flutter#134867)
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@engine-flutter-autoroll @HugoOlthof
Roll Flutter from 1b18b1327a59 to b7d0e8c9ed70 (11 revisions) (flutte…
4915b5c 
…r#4944)

flutter/flutter@1b18b13...b7d0e8c

2023-09-18 engine-flutter-autoroll@skia.org Roll Packages from bc8c2f2 to d4e2454 (6 revisions) (flutter/flutter#134945)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 4107ee0dc8b1 to be7a039c5451 (1 revision) (flutter/flutter#134937)
2023-09-18 dacoharkes@google.com Native assets support for Linux (flutter/flutter#134031)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 77965cabbaf9 to 4107ee0dc8b1 (1 revision) (flutter/flutter#134927)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 75e5ba3c4671 to 77965cabbaf9 (1 revision) (flutter/flutter#134925)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 573e44b91887 to 75e5ba3c4671 (1 revision) (flutter/flutter#134923)
2023-09-18 104349824+huycozy@users.noreply.github.com Improve DropdownMenu sample code for requestFocusOnTap on mobile platforms (flutter/flutter#134867)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 51a402860f5b to 573e44b91887 (1 revision) (flutter/flutter#134920)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from f53681fb2220 to 51a402860f5b (1 revision) (flutter/flutter#134918)
2023-09-18 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9f3e46a21e51 to f53681fb2220 (1 revision) (flutter/flutter#134916)
2023-09-17 engine-flutter-autoroll@skia.org Roll Flutter Engine from 57a4ff3c7ff8 to 9f3e46a21e51 (1 revision) (flutter/flutter#134909)

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 dit@google.com,rmistry@google.com,stuartmorgan@google.com 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://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuncCccccc QuncCccccc QuncCccccc approved these changes

Assignees

@huycozy huycozy

Labels
autosubmit Merge PR when tree becomes green via auto submit App d: api docs Issues with https://api.flutter.dev/ d: examples Sample code and demos f: focus Focus traversal, gaining or losing focus framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
2 participants
@huycozy @QuncCccccc