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.

Sign up for GitHub

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

Jump to bottom

message_view: For message sent by current user, always scroll into view. #29398

Merged
merged 1 commit into from
Apr 11, 2024
Merged

message_view: For message sent by current user, always scroll into view. #29398

merged 1 commit into from
Apr 11, 2024

Conversation

N-Shar-ma
Copy link
Collaborator

@N-Shar-ma N-Shar-ma commented Mar 21, 2024 •
edited
Loading

Now whenever the bottom of the feed is visible, we always scroll to the message/s just sent by the current user, updating the selected message if necessary.

Scroll behaviour for messages sent by other users remains unchanged.

Fixes: #23298.

Screenshots and screen captures:
Before sending:
image

New message scrolled into view on sending:
image

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@zulipbot zulipbot added size: L area: message feed (scrolling) Scroll behavior, performance, and side-effects (marking as read) post release Issues to focus attention on after the current major release priority: high UI experiment Design direction to be evaluated labels Mar 21, 2024
@zulipbot
Copy link
Member

Hello @zulip/design members, this pull request was labeled with the "UI experiment" label, so you may want to check it out!

@N-Shar-ma N-Shar-ma mentioned this pull request Mar 21, 2024
Closed
@zulipbot zulipbot added size: S and removed size: L labels Mar 21, 2024
@N-Shar-ma N-Shar-ma changed the title message_view: Always autoscroll to new message unless scrolled up. message_view: For message sent by current user, always scroll into view. Mar 21, 2024
@alya
Copy link
Contributor

alya commented Mar 25, 2024 •
edited
Loading

Hm, how does it determine how far to scroll? E.g., I got this much blank space sometimes, but a much narrower strip other times.

Screenshot 2024-03-25 at 4 35 53 PM

This is after sending a message that was long enough to stretch the compose box, in case that's relevant.

@timabbott
Copy link
Member

timabbott commented Mar 26, 2024 •
edited
Loading

This looks like it correctly implements what we specified.

I believe it scrolls an amount equal to the height of the newly added messages.

@N-Shar-ma
Copy link
Collaborator Author

Just bumping this up @alya

@alya
Copy link
Contributor

alya commented Apr 1, 2024

Hm, this isn't what I was imagining, but it actually feels pretty reasonable when I test it. Let's try it out on CZO and go from there.

@alya alya added the chat.zulip.org review Added by maintainers when a PR requires chat.zulip.org testing to proceed. label Apr 1, 2024
@N-Shar-ma
message_view: For message sent by current user, always scroll into view.
360f636 
Now whenever the bottom of the feed is visible, we always scroll to the
message/s just sent by the current user, updating the selected message
if necessary.

Scroll behaviour for messages sent by other users remains unchanged.

Fixes: zulip#23298.
@timabbott timabbott added the deployed on chat.zulip.org Added by maintainers when a PR is currently being tested on chat.zulip.org. label Apr 2, 2024
@timabbott timabbott merged commit 358bb6b into zulip:main Apr 11, 2024
6 checks passed
@timabbott
Copy link
Member

Merged, thanks @N-Shar-ma!

This was referenced Apr 11, 2024
Closed
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
area: message feed (scrolling) Scroll behavior, performance, and side-effects (marking as read) chat.zulip.org review Added by maintainers when a PR requires chat.zulip.org testing to proceed. deployed on chat.zulip.org Added by maintainers when a PR is currently being tested on chat.zulip.org. post release Issues to focus attention on after the current major release priority: high size: S UI experiment Design direction to be evaluated
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Scroll to sent message when bottom of the screen is in view
4 participants
@N-Shar-ma @zulipbot @alya @timabbott