Fix autocomplete options height #80187

maheshj01 · 2021-04-10T09:12:59Z

This pull request fixes the Autocomplete Widget's options height issue #79227 and adds a maxOptionsHeight property allowing user to configure the maximum height the optionsViewBuilder can take.

Fixes #79227

flutter-dashboard · 2021-04-10T09:13:01Z

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 on the #hackers channel in Chat.

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.

maheshj01 · 2021-04-10T09:24:21Z

@justinmc let me know your thoughts on this PR would be happy to make the changes :)
Also, I couldn't understand from the logs why the flutter-build failed.

maheshj01 · 2021-04-13T14:40:33Z

@shihaohong can you please review this PR

shihaohong

This PR would need tests to verify that the updated, intended behaviors are reflected.

maheshj01 · 2021-04-14T07:03:30Z

This PR would need tests to verify that the updated, intended behaviors are reflected.

Thanks, I will add the tests to support this PR

maheshj01 · 2021-04-14T12:56:59Z

@shihaohong I am not sure if I have to write tests in widgets/autocomplete_test.dart as well

…issue-79227

pedromassango

@maheshmnj It looks like there is some unrelated changes caused by your IDE/code editor, that is causing the analyzer test to fail, you have to revert those changes, it is basicaly remove empty spaces (custom formats) added by your IDE/code editor.

packages/flutter/lib/src/material/autocomplete.dart

packages/flutter/test/material/autocomplete_test.dart

justinmc

The tests are failing due to an improperly formatted test (see my comment).

For flutter_build, you don't have to worry about it. It represents the status of the current Flutter build (without this PR). We only allow PRs to be merged when it is green. If it's red, someone is working on it, and you can just wait for it to turn green.

packages/flutter/test/material/autocomplete_test.dart

packages/flutter/lib/src/material/autocomplete.dart

shihaohong

Some comments about the test file's formatting and the test setup and clarity.

packages/flutter/lib/src/material/autocomplete.dart

packages/flutter/test/material/autocomplete_test.dart

shihaohong · 2021-05-03T14:12:20Z

The analyzer check is also failing:

Warning: transitive closure contained non-allowlisted packages:
[frontend_server_client]
See dev/bots/allowlist.dart for instructions on how to update the package allowlist.

It might be unrelated to your change, so maybe rebase your branch with the master branch and see if it resolved the error failure.

maheshj01 · 2021-05-03T17:53:33Z

Thank you so much for the detailed review @shihaohong

packages/flutter/test/material/autocomplete_test.dart

…issue-79227

maheshj01 · 2021-05-05T04:55:56Z

@shihaohong I am not exactly sure why the Windows build_tests are failing looks like they are timing out.
I have formatted the code Please review.

shihaohong

LGTM

shihaohong · 2021-05-05T09:03:27Z

The windows tests seem to be unrelated to your change and is an infra failure (I think the related issue is #81837). We'll have to wait for it to be resolved before we can merge this PR

maheshj01 · 2021-05-05T15:12:04Z

The windows tests seem to be unrelated to your change and is an infra failure (I think the related issue is #81837). We'll have to wait for it to be resolved before we can merge this PR

sure thanks again for the review

pedromassango

LGTM

maheshj01 · 2021-05-18T13:03:19Z

@shihaohong looks like #81837 is resolved. do I need to merge the latest master? In order for windowsbuild_tests_2_3 to pass

pedromassango · 2021-05-20T10:24:39Z

@maheshmnj I think so

fluttergithubbot · 2021-05-20T10:29:02Z

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

The status or check suite Windows build_tests_2_3 has failed. Please fix the issues identified (or deflake) before re-applying this label.

…issue-79227

fix issue #79227 and add configurable maxOptionsHeight property

647c641

flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Apr 10, 2021

google-cla bot added the cla: yes label Apr 10, 2021

shihaohong reviewed Apr 14, 2021

View reviewed changes

maheshj01 added 3 commits April 14, 2021 14:48

update docs

5de1d81

tests for options height

ab5c0a3

fix warning always specify types

d5ef395

Merge branch 'master' of https://github.com/flutter/flutter into fix/…

2112e3a

…issue-79227

pedromassango requested changes Apr 19, 2021

View reviewed changes

packages/flutter/lib/src/material/autocomplete.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

maheshj01 added 2 commits April 19, 2021 21:14

fix: capitalize doc comments

c579444

format: remove extraneous space

73dada8

maheshj01 requested a review from pedromassango April 19, 2021 16:55

justinmc reviewed Apr 19, 2021

View reviewed changes

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

fix:remove nested test

f606f85

pedromassango reviewed Apr 20, 2021

View reviewed changes

packages/flutter/lib/src/material/autocomplete.dart Outdated Show resolved Hide resolved

pedromassango reviewed Apr 20, 2021

View reviewed changes

packages/flutter/lib/src/material/autocomplete.dart Outdated Show resolved Hide resolved

pedromassango reviewed Apr 20, 2021

View reviewed changes

packages/flutter/lib/src/material/autocomplete.dart Outdated Show resolved Hide resolved

add period and white space around = sign

50ce3ae

shihaohong suggested changes Apr 23, 2021

View reviewed changes

maheshj01 added 5 commits April 23, 2021 16:10

renamed property to optionsMaxHeight

18904d8

add more context to comments

3da858b

add clear description to test

3860d1b

remove unnecessary variable

425e6d7

made function private and improved comments

c6218f6

maheshj01 added 2 commits April 23, 2021 17:06

use block comments

1b07b75

remove trailing space

d36e603

maheshj01 requested a review from shihaohong April 29, 2021 12:53

shihaohong reviewed May 3, 2021

View reviewed changes

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

maheshj01 added 2 commits May 3, 2021 21:08

revert unrelated change

5cb456a

Merge branch 'master' into fix/issue-79227

b9068e5

shihaohong reviewed May 4, 2021

View reviewed changes

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

chore: format code

0d056de

maheshj01 requested a review from shihaohong May 4, 2021 16:35

shihaohong reviewed May 4, 2021

View reviewed changes

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

packages/flutter/test/material/autocomplete_test.dart Outdated Show resolved Hide resolved

maheshj01 added 4 commits May 5, 2021 06:41

adjust indentation

86cea46

move child at last parameter

8f41692

remove trailing space

911f972

Merge branch 'master' of https://github.com/flutter/flutter into fix/…

263a2d6

…issue-79227

maheshj01 requested a review from shihaohong May 5, 2021 04:57

shihaohong approved these changes May 5, 2021

View reviewed changes

pedromassango approved these changes May 5, 2021

View reviewed changes

pedromassango added the waiting for tree to go green label May 20, 2021

fluttergithubbot removed the waiting for tree to go green label May 20, 2021

Merge branch 'master' of https://github.com/flutter/flutter into fix/…

d975f74

…issue-79227

pedromassango added the waiting for tree to go green label May 20, 2021

fluttergithubbot merged commit 238bc4b into flutter:master May 20, 2021

Fix autocomplete options height #80187

Fix autocomplete options height #80187

Uh oh!

Conversation

maheshj01 commented Apr 10, 2021 • edited by pedromassango Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Apr 10, 2021

Uh oh!

maheshj01 commented Apr 10, 2021

Uh oh!

maheshj01 commented Apr 13, 2021

Uh oh!

shihaohong left a comment

Choose a reason for hiding this comment

Uh oh!

maheshj01 commented Apr 14, 2021

Uh oh!

maheshj01 commented Apr 14, 2021

Uh oh!

pedromassango left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shihaohong left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shihaohong commented May 3, 2021

Uh oh!

maheshj01 commented May 3, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maheshj01 commented May 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shihaohong left a comment

Choose a reason for hiding this comment

Uh oh!

shihaohong commented May 5, 2021

Uh oh!

maheshj01 commented May 5, 2021

Uh oh!

pedromassango left a comment

Choose a reason for hiding this comment

Uh oh!

maheshj01 commented May 18, 2021

Uh oh!

pedromassango commented May 20, 2021

Uh oh!

fluttergithubbot commented May 20, 2021

Uh oh!

Uh oh!

maheshj01 commented Apr 10, 2021 •

edited by pedromassango

Loading

shihaohong left a comment •

edited

Loading

maheshj01 commented May 5, 2021 •

edited

Loading