page: Split empty-page placeholder messages into header and message
#1946
Conversation
chrisbobbe
added
a-design
|
The bigger text when we only have one sentence looks good to me. |
|
Thanks for the review! Also maybe worth mentioning: the graphics in the Figma are tracked as #1551. |
rajveermalviya
added
integration review
| }) : assert( | ||
| (message != null) | ||
| ^ (messageWithLinkMarkup != null && onTapLink != null)); |
There was a problem hiding this comment.
This assert seems still relevant, right?
And a similar one for the header.
There was a problem hiding this comment.
Ah I see, rather it should apply only to the header.
lib/widgets/page.dart
Outdated
| return null; | ||
| } |
There was a problem hiding this comment.
This case would be a straightforward bug at the caller. So instead of having extra logic here (including in build) to handle it, it'd be better to just use ! and end up throwing in that case.
chrisbobbe
force-pushed
the
pr-split-placeholder-header-message
branch
from
51c196a to
70cc1b3
Compare
|
Thanks for the review! Revision pushed. |
| style: _headerStyle(context), | ||
| header!); | ||
| } | ||
| return TextWithLink( |
There was a problem hiding this comment.
(#1946 (comment) cont'd)
Ah, and then this method's caller can be simplified by having this return non-nullable Widget 🙂
chrisbobbe
force-pushed
the
pr-split-placeholder-header-message
branch
from
70cc1b3 to
a3f5b81
Compare
|
Indeed, thanks—revision pushed. |
Following a new Figma frame that specifies larger text for the first part: https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=11194-18392&m=dev
gnprice
force-pushed
the
pr-split-placeholder-header-message
branch
from
a3f5b81 to
db64354
Compare
|
Thanks! Looks good; merging. |
4 participants
Following a new Figma frame that specifies larger text for the first part:
https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=11194-18392&m=dev
cc @alya @terpimost
👉 When there's just one thing to say (like "There are no messages here."), do we want that text to be big or small? I've chosen big, here, but that might not actually be intended in the Figma? Figma screenshot:
(This comes from rebasing/developing part of #1640.)
Note changed "the" to "a" on the "Direct messages" page.