Repository navigation
Add CheckedPopupMenuitem, update the Stocks demo - #2216
Conversation
| ), | ||
| new PopupMenuItem( | ||
| disabled: true, | ||
| isDisabled: true, |
There was a problem hiding this comment.
Can we change this to "enabled: false"? The Web has had a number of issues due to having booleans that are negative adjectives.
There was a problem hiding this comment.
It's a pity we can't use the onPressed != null approach.
There was a problem hiding this comment.
Can we change this to "enabled: false"? The Web has had a number of issues due to having booleans that are negative adjectives.
Sure. I've been using isFoo for boolean properties, so isEnabled?
There was a problem hiding this comment.
maybe value != null?
I have mixed feelings about this kind of overloading since developers are unlikely to guess that it's how one disables an item. But if you really think it's a good idea, OK.
There was a problem hiding this comment.
let's stick with the boolean.
As far as isFoo vs foo, for widget constructor arguments, we don't generally do the is prefix. isFoo is usually reserved for getters that examine state.
There was a problem hiding this comment.
OK. I'll de-prefix ListItem's isThreeLine, isDisabled, and isDense as well.
There was a problem hiding this comment.
"isThreeLine" is probably better than "threeLine" since the latter isn't an adjective. Generally I think my intuition is that arguments that are booleans should be adjective, and arguments that take other types should be nouns. (length:3 -> "the length is 3" vs dense:true -> "this is dense")
There was a problem hiding this comment.
I've changed isDense to dense. Sadly, I've just noticed that Input also supports dense layout. It's property is called isDense. Sigh.
|
LGTM. Sorry about not catching the isFoo thing in earlier patches. |
Add CheckedPopupMenuitem, update the Stocks demo
Android: Use android.arch.lifecycle instead of androidx.lifecycle:lifecycle in build.gradle to support apps that has not been migrated to AndroidX.
2 participants
Using PopupMenuButton and CheckedPopupMenuItem in the stocks demo.
Fixes #2200
Also: