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

Fix issue with sending messages. Scroll to bottom on send #1048

Merged
merged 1 commit into from Aug 16, 2017

Conversation

borisyankov
Copy link
Contributor

No description provided.

@smarx
Copy link

smarx commented Aug 16, 2017

Automated message from Dropbox CLA bot

@borisyankov, it looks like you've already signed the Dropbox CLA. Thanks!

@borisyankov borisyankov merged commit cd49d9b into zulip:master Aug 16, 2017
@timabbott
Copy link
Sponsor Member

Quick question about this -- if you're like 50 messages above the bottom and send a message, does this scroll you all the way down, or do we just scroll down when you were near the bottom in the first place?

Also, presumably we want to have most logic we have around scrolling down on send trigger on receive, not send, since otherwise you'll still miss things other people send, right?

@borisyankov
Copy link
Contributor Author

Yeah, it does scroll you all the way down.
We already have @kunall17 outbox implementation where the message is locally prerendered!

@timabbott
Copy link
Sponsor Member

OK. I think that's absolutely the right behavior if you're near the bottom, but the wrong behavior when you're more than a screen's height from the bottom of the feed. Otherwise, sending a quick reply will lose your place (when catching up, I often find myself replying to several messages within a feed as I read through it).

If you're not near the bottom, we may want a little notice that your message sent successfully and is further below.

@borisyankov
Copy link
Contributor Author

Sounds good. And the notice, optionally, when clicked might scroll you to the bottom too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants