Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stream and topic narrows #147

Merged
merged 8 commits into from
Jun 2, 2023
Merged

Add stream and topic narrows #147

merged 8 commits into from
Jun 2, 2023

Conversation

gnprice
Copy link
Member

@gnprice gnprice commented Jun 1, 2023

This is enough that the app starts to be usable for more real-world use cases! Yesterday when I was on the go and had a real, non-test message I wanted to send, I had a draft of this branch already on my phone, and realized that it meant I could use zulip-flutter to send the message instead of zulip-mobile, so I did. That also marked the first time I'd logged into the Flutter app with my main account (rather than test accounts).

(Specifically, if what you want to do is send a message in some stream you've recently gotten a message in, you can do it: go to "All messages", find a message to that stream, tap the stream part of the header to get to the stream narrow, then use the compose box. With #144 that will work with the topic narrow too, with #142 it'll broaden to DMs, and of course we'll also be adding some more-direct navigation options before too long.)

Fixes: #72

gnprice added 8 commits May 31, 2023 18:06
Up to now we've provided navigation only to the all-messages narrow,
and we've given that screen a compose box with a hack that has it
send only to `#test here`.

We're going to remove that hack and put no compose box on that screen.
To prepare for that, we'll want some other way to quickly get to a
screen with a compose box, for testing.  Do that with the actual
stream narrow for `#test here`.
Now that one can navigate to the actual stream narrow for the
`#test here` stream on chat.zulip.org, there's no longer a need
for the hack that offered that stream's compose box on the
all-messages narrow's screen.

And then with that hack gone, the send button works correctly
on any realm; so we can drop the condition that made it refuse
to operate outside of chat.zulip.org.
@gnprice gnprice added the a-msglist The message-list screen, except what's label:a-content label Jun 1, 2023
@chrisbobbe
Copy link
Collaborator

Thanks! This looks good to me.

For when and how to present a compose box, I think we could do some more design work in a later phase, to not re-introduce issues like these:

But this seems fine to merge so we don't block on that design work, and we can come back to it later.


For example, one way I think it could work is:

  • De-emphasize interleaved stream views in the navigation
  • For topic and DM-conversation views, give a compose box that doesn't offer changing the destination conversation (topic or recipients, respectively).
  • For other views (e.g. all-messages; stream views; arbitrary narrows), don't show a compose box until the user asks for one, with "reply" or "quote and reply" on a specific message. The compose box will send to that message's conversation, without offering a change of destination, since that could lead to confusion and accidents.
  • Offer a "new topic" UI: Explicit "New topic" UI zulip-mobile#4379

@chrisbobbe
Copy link
Collaborator

chrisbobbe commented Jun 2, 2023

Oh hmm, one thing I did notice when testing this—not introduced in this PR, so OK to address as a followup—

When the list of messages takes less than a screenful, it aligns to the bottom of the area instead of the top. Does that look odd to you too?

@gnprice
Copy link
Member Author

gnprice commented Jun 2, 2023

Thanks for the review!

For when and how to present a compose box, I think we could do some more design work in a later phase, to not re-introduce issues like these:

Sure. Filed #149 to track that.

When the list of messages takes less than a screenful, it aligns to the bottom of the area instead of the top. Does that look odd to you too?

Hmm, good observation.

That wasn't an explicit choice; it just falls out naturally from the implementation.

That behavior is different from what's in zulip-mobile, and what's in Zulip web. On the other hand, it's the same behavior I think one sees most often in other messaging apps — in particular I just checked the stock Android SMS client, and Discord, and I recall Slack does the same. It's more consistent in its own way, in that the latest messages are always at the very bottom.

I think we can run with this behavior for now, and see what feedback we get.

@gnprice gnprice merged commit a757620 into zulip:main Jun 2, 2023
@gnprice gnprice deleted the pr-narrows branch June 2, 2023 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-msglist The message-list screen, except what's label:a-content
Projects
None yet
Development

Successfully merging this pull request may close these issues.

msglist: Offer stream and topic narrows
2 participants