msglist [nfc]: Remove one Center widget (because no-op); comment to explain another
#1042
Conversation
The Center's child is a Column with the message list and possibly a compose box. That Column expresses that it wants to fill all the available space (which I agree seems desirable), so there's only one position it can have in that space. Asking it to be centered there is a harmless no-op but is confusing when reading the code, so, stop doing so. This Center looks like a relic from the very early prototype in late 2022; see commit b22ef67.
Looks like I explained the Center widget in the commit message that added it -- ef02ee4 -- but I think the comment is helpful. At first, it looked like another case of a no-op Center widget like in the previous commit, but it turns out it does have a desirable effect in the horizontal axis.
| @@ -467,6 +467,7 @@ class _MessageListState extends State<MessageList> with PerAccountStoreAwareStat | |||
| // will have a `MediaQuery.removePadding` with `removeBottom: true`. | |||
| bottom: false, | |||
|
|
|||
| // (center horizontally on wide screens; a no-op in the vertical direction) | |||
There was a problem hiding this comment.
Do you have a screenshot when that happens? I wasn't able to reproduce the effect if I remove Center on my devices:
Linux
Android
Maybe I looked at the wrong place? Oh! I see, I can reproduce that when the compose box is absent in the combined feed, because in that case the message list is the only child in the Column on MessageListPage, and the message list doesn't fill horizontally like ComposeBox does, if without Center.
Details
There was a problem hiding this comment.
If we remove the Center here, and keep the one on MessageListPage, the combined feed looks fine. Because Center specifically deals with sizing the widget within a Scaffold, it might be clearer to remove the Center here, and comment on the Center on MessageListPage.
SizedBox.expand might be a good alternative, because Column can center its children.
No description provided.