msglist: Friendlier placeholder text when narrow has no messages (simple version) #1650
Conversation
|
There was a problem hiding this comment.
Thanks @chrisbobbe! Just one small comment, otherwise LGTM.
| @@ -148,40 +148,6 @@ class _HomePageState extends State<HomePage> { | |||
| } | |||
There was a problem hiding this comment.
page: Move no-content placeholder widget to page.dart, from home.dart
Should this commit be marked as nfc?
chrisbobbe
force-pushed
the
pr-empty-msglist-simple-placeholder
branch
from
9ed5862 to
1fc69d5
Compare
|
Thanks! Revision pushed, and I'll mark for Greg's review. |
chrisbobbe
added
integration review
chrisbobbe
force-pushed
the
pr-empty-msglist-simple-placeholder
branch
from
1fc69d5 to
6cfdd1d
Compare
|
(Updated just to add a test that I wrote for #1640 but also belongs here.) |
| ).findsOne(); | ||
| }); | ||
|
|
||
| testWidgets('when `messages` empty but `outboxMessages` not empty, show outboxes, not placeholder', (tester) async { |
There was a problem hiding this comment.
Yeah, good thought to test this.
| await tester.enterText(contentInputFinder, 'asdfjkl;'); | ||
| await tester.tap(find.byIcon(ZulipIcons.send)); | ||
| await tester.pump(kLocalEchoDebounceDuration); | ||
|
|
||
| check(findPlaceholder).findsNothing(); | ||
| check(find.text('asdfjkl;')).findsOne(); |
There was a problem hiding this comment.
And this is a nice user-focused structure for the test.
lib/widgets/message_list.dart
Outdated
| if (model.haveNewest && model.haveOldest | ||
| && model.messages.isEmpty && model.outboxMessages.isEmpty) { |
There was a problem hiding this comment.
Maybe test model.items is empty, rather than messages and outboxMessages separately?
| if (model.haveNewest && model.haveOldest | |
| && model.messages.isEmpty && model.outboxMessages.isEmpty) { | |
| if (model.items.isEmpty && model.haveNewest && model.haveOldest) { |
That's the layer the rest of this widget's logic works at: it operates on items and doesn't look at the messages or outboxMessages lists directly.
If in some future we add another way there can be items in the list, with neither messages nor outbox-messages, and don't edit this, then that would mean it'd show the start and end caps, and the items in between. I think that's probably the right outcome: the items are there in the view-model, so should be shown. (If they shouldn't be shown, they shouldn't be in the view-model.)
chrisbobbe
force-pushed
the
pr-empty-msglist-simple-placeholder
branch
from
6cfdd1d to
d526799
Compare
|
Thanks! Revision pushed. |
…e.dart We can reuse this for the empty message list.
Fixes zulip#1555. For now, the text simply says "There are no messages here." We'll add per-narrow logic later, but this is an improvement over the current appearance which just says "No earlier messages." (Earlier than what?) To support being used in the message-list page (in addition to Inbox, etc.), the placeholder widget only needs small changes, it turns out.
gnprice
force-pushed
the
pr-empty-msglist-simple-placeholder
branch
from
d526799 to
8749817
Compare
|
Thanks! Looks good; merging. |
Splitting this out from #1640, so it can be merged soon.
Fixes #1555.
For now, the text simply says "There are no messages here." We'll add per-narrow logic later, but this is an improvement over the current appearance which just says "No earlier messages." (Earlier than what?)