The Wayback Machine - https://web.archive.org/web/20210915203906/https://github.com/flutter/flutter/pull/76493
Skip to content

/ flutter Public

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.

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

Add menuMaxHeight for DropdownButton #76493

Merged
merged 6 commits into from Mar 9, 2021
Merged

Add menuMaxHeight for DropdownButton #76493

merged 6 commits into from Mar 9, 2021

Conversation

@YeungKC
Copy link
Member

@YeungKC YeungKC commented Feb 21, 2021 •
edited

To satisfy our usage, I added menuMaxHeight. By default, the height of the menu does not change.

Some people want to control it by padding, some want to set the maximum height, some want to control it by height percentage, but if only the maximum height limit is increased, other needs can be calculated by menuMaxHeight.

close #23865

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 listed at least one issue that this PR fixes in the description above.
  • I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt.
  • I updated/added relevant documentation (doc comments with ///).
  • I signed the CLA.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.
@YeungKC
feat: init
8b6a383
@google-cla google-cla bot added the cla: yes label Feb 21, 2021
@YeungKC YeungKC requested review from shihaohong and HansMuller Feb 21, 2021
shihaohong
shihaohong reviewed Feb 23, 2021
View changes
packages/flutter/lib/src/material/dropdown.dart Outdated
/// The [menuMaxHeight] should be less than or equal to the maximum height of
/// one less row, otherwise it is invalid.
Copy link
Contributor

@shihaohong shihaohong Feb 23, 2021

Could you clarify what you mean here by one less row? Does this mean that if you have a menu with 5 items, it can be the maximum height of the height of 4 items? Why not up to 5 items (maybe a height that shows half an item)?

Copy link
Contributor

@shihaohong shihaohong Feb 23, 2021

Ahh, you mean one less row compared to the view itself. Maybe clarify that, because my first thought was one less row relative to the maximum height that the menu can be, assuming the entire menu fit on the screen.

shihaohong
shihaohong reviewed Feb 23, 2021
View changes
packages/flutter/lib/src/material/dropdown.dart Outdated
/// The maximum height of the menu.
///
/// The [menuMaxHeight] should be less than or equal to the maximum height of
/// one less row, otherwise it is invalid.
Copy link
Contributor

@shihaohong shihaohong Feb 23, 2021

You may also want to specify that if the height is set to a value greater than the allowed max height, then the allowed max height is used by default and explain why that is the case in the documentation as well. This is actually pretty well described here, so just reuse it to some degree

shihaohong
shihaohong reviewed Feb 23, 2021
View changes
Copy link
Contributor

@shihaohong shihaohong left a comment

Looks good! It just needs some documentation to help users understand how to use this API better!

@shihaohong shihaohong mentioned this pull request Feb 23, 2021
Closed
@YeungKC
refactor: Improved doc.
6a28312
Update packages/flutter/lib/src/material/dropdown.dart
c41ee54 
Nits on documentation
shihaohong
shihaohong approved these changes Feb 23, 2021
View changes
Copy link
Contributor

@shihaohong shihaohong left a comment

LGTM, with a little bit more of updating to the API docs. Thanks for the fix!

Shi-Hao Hong added 2 commits Mar 9, 2021
Merge branch 'master' into Add_menMaxHeight_for_DropdownButton
85ffd2d
Update dropdown.dart
d0b654e 
Remove trailing whitespace
@shihaohong
Copy link
Contributor

@shihaohong shihaohong commented Mar 9, 2021

Had to fix some trailing whitespace issues and a merge conflict. Should be good to go again as long as the bots are happy!

@fluttergithubbot
Copy link
Contributor

@fluttergithubbot fluttergithubbot commented Mar 9, 2021

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Linux analyze has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite Mac framework_tests has failed. Please fix the issues identified (or deflake) before re-applying this label.
  • The status or check suite analyze-linux has failed. Please fix the issues identified (or deflake) before re-applying this label.

Update dropdown_test.dart
232ebc4 
One more trailing whitespace
@fluttergithubbot fluttergithubbot merged commit e352e08 into flutter:master Mar 9, 2021
34 checks passed
34 checks passed
@flutter-github-sync
Google testing Google testing passed!
Details
@flutter-dashboard[bot]
Linux analyze
Details
@flutter-dashboard[bot]
Linux build_tests
Details
@flutter-dashboard[bot]
Linux customer_testing
Details
@flutter-dashboard[bot]
Linux docs
Details
@flutter-dashboard[bot]
Linux firebase_abstract_method_smoke_test
Details
@flutter-dashboard[bot]
Linux firebase_android_embedding_v2_smoke_test
Details
@flutter-dashboard[bot]
Linux firebase_release_smoke_test
Details
@flutter-dashboard[bot]
Linux flutter_plugins
Details
@flutter-dashboard[bot]
Linux framework_tests
Details
@flutter-dashboard[bot]
Linux fuchsia_precache
Details
@flutter-dashboard[bot]
Linux web_e2e_test
Details
@flutter-dashboard[bot]
Linux web_integration_tests
Details
@flutter-dashboard[bot]
Linux web_long_running_tests
Details
@flutter-dashboard[bot]
Linux web_smoke_test
Details
@flutter-dashboard[bot]
Linux web_tests
Details
@flutter-dashboard[bot]
Mac build_tests
Details
@flutter-dashboard[bot]
Mac customer_testing
Details
@flutter-dashboard[bot]
Mac framework_tests
Details
@wip[bot]
WIP Ready for review
Details
@flutter-dashboard[bot]
Windows build_tests
Details
@flutter-dashboard[bot]
Windows customer_testing
Details
@flutter-dashboard[bot]
Windows framework_tests
Details
@cirrus-ci[bot]
analyze-linux Task Summary
Details
@google-cla[bot]
cla/google All necessary CLAs are signed
@cirrus-ci[bot]
customer_testing-linux Task Summary
Details
@cirrus-ci[bot]
docs-linux Task Summary
Details
@flutter-dashboard[bot]
flutter-build Flutter build is currently broken. Please do not merge this PR unless it contains a fix to the broken build.
Details
@flutter-dashboard[bot]
flutter-gold All golden file tests have passed.
Details
@cirrus-ci[bot]
framework_tests-libraries-linux Task Summary
Details
@cirrus-ci[bot]
framework_tests-misc-linux Task Summary
Details
@cirrus-ci[bot]
framework_tests-widgets-linux Task Summary
Details
@cirrus-ci[bot]
web_integration_tests Task Summary
Details
@cirrus-ci[bot]
web_smoke_test Task Summary
Details
@YeungKC
Copy link
Member Author

@YeungKC YeungKC commented Mar 9, 2021

thx

@mtunahansarioglu
Copy link

@mtunahansarioglu mtunahansarioglu commented Mar 14, 2021

It's really good to see this problem resolved!

@Okladnoj
Copy link

@Okladnoj Okladnoj commented Apr 15, 2021 •
edited

In which version of flutter will this feature appear?

@GusRodrigues86
Copy link

@GusRodrigues86 GusRodrigues86 commented Jun 16, 2021

In which version of flutter will this feature appear?

I see available for use in flutter 2.2.1+

@kentomiya89 kentomiya89 mentioned this pull request Jul 24, 2021
Open
@xu-baolin xu-baolin mentioned this pull request Aug 31, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shihaohong shihaohong

@HansMuller HansMuller

Assignees
No one assigned
Labels
cla: yes f: material design framework waiting for tree to go green
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Option to limit size of DropDown from DropDownButton
6 participants
@YeungKC @shihaohong @fluttergithubbot @mtunahansarioglu @Okladnoj @GusRodrigues86