Repository navigation
Reprise: Dropdown Menu layout respects menu items intrinsic sizes - #42033
Conversation
fluttergithubbot
added
p: material_ui
|
Original PR for posterity: #41120 |
shihaohong
left a comment
shihaohong
left a comment
There was a problem hiding this comment.
A few nits for code comments. Is it expected that when itemHeights is set to null and the menu is too tall to fit the screen, the scroll offset is always set to zero? If so, we should document that somewhere.
There was a problem hiding this comment.
| // for the ListView that contains he menu items. The vertical center of the | |
| // for the ListView that contains the menu items. The vertical center of the |
There was a problem hiding this comment.
| // out, only works if the item heights are fixed. This works out when | |
| // out. It only works if the item heights are fixed. This works out when |
There was a problem hiding this comment.
Could we also add a description for what would happen if the item heights aren't fixed? ie:
| // the item heights are defined by kMinInteractiveDimension. | |
| // the item heights are defined by kMinInteractiveDimension. If | |
| // itemHeight is set to null, initialScrollOffset will be set to 0. |
There was a problem hiding this comment.
| // The Each item appears twice, once in the menu and once | |
| // Each item appears twice, once in the menu and once |
There was a problem hiding this comment.
This seems to still be here
There was a problem hiding this comment.
I think this comment belongs a few lines down (right above hint)
There was a problem hiding this comment.
I copied that from the original code, however I agree that it belongs above the Stack, so I moved it.
There was a problem hiding this comment.
Is it worth mentioning here that if the preferred menu height is too tall for the screen and itemHeights = null, the scroll offset will not be properly set and always be at zero?
There was a problem hiding this comment.
Yes, sadly yes. Updated the doc to explain this.
efe3262 to
ff0203a
Compare
shihaohong
left a comment
shihaohong
left a comment
There was a problem hiding this comment.
LGTM with a few nits.
Also, it seems that if the options are too tall, the dropdown menu doesn't scroll to show the selected item. This might be the expected behavior. If not, we should fix it, but that's probably outside of the scope of this PR:
- Select item that is at bottom of the menu list
- Note that the dropdown always starts at the top of the menu instead of starting where the selected item is.
See gist for code, and see gif with current behavior:
There was a problem hiding this comment.
This seems to still be here
There was a problem hiding this comment.
| // treating the items as if their heights were all equal to kMinInteractveDimension. | |
| // treating the items as if their heights were all equal to kMinInteractiveDimension. |
There was a problem hiding this comment.
| // Otherwise the initialScrollOffset is just an rough approximation based on | |
| // Otherwise the initialScrollOffset is just a rough approximation based on |
ff0203a to
2da8883
Compare
|
The reason an itemHeight:null dropdown menu with 4 very large menu items doesn't automatically scroll is that the dropdown doesn't know that the items need to be scrollable until after layout, which is too late. In other words, based on the initial assumption that all of the items have height=48, the menu appears to fit on the screen. |
|
@shihaohong Any idea when this update will be available on "stable" branch? |
…utter#42033) * Reprise: Dropdown Menu layout respects menu items intrinsic sizes * updated per review feedback * updated per review feedback
5 participants
Fixes #40773.
This is an updated version of #41120.
Previously, dropdown menu items had a fixed height of 48 (kMinInteractiveDimension). Now, if DropdownButton.itemHeight is null, then menu items have a minimum height of kMinInteractiveDimension and no maximum height.
This test case demos dropdown menus with variable height items. It also uses the new selectedItemBuilder feature to render compact versions of the selected items within the dropdown button. If the selectedItemBuilder is not specified, then the DropdownButton's height will be as big as the largest menu item.