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

improvement: better scroll for jumpToMessage #4286

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

WofWca
Copy link
Collaborator

@WofWca WofWca commented Oct 28, 2024

Add scrollIntoViewArg parameter to jumpToMessage
and specify different values for it depending on context.

More specifically, prefer scrolling the target message
to the center, e.g. when showing the first unread message.
Prior to this commit, we'd scroll the first unread message
such that it's at the bottom of the message list,
which can be considered a regression, which was introduced in
5f0efe1.

Closes #4284.

I recommend reviewing commit by commit.

TODO:

@WofWca WofWca force-pushed the wofwca/fix-scroll-to-first-unread branch from 77098d7 to 49eac14 Compare October 28, 2024 14:40
@WofWca WofWca marked this pull request as ready for review October 28, 2024 14:42
@WofWca WofWca added the ui/ux UI/UX related issues label Oct 28, 2024
@WofWca WofWca force-pushed the wofwca/fix-scroll-to-first-unread branch from 49eac14 to 4fd1d57 Compare October 28, 2024 14:43
Copy link
Contributor

@nicodh nicodh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By reading and talking to wofwca

Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks good, didn't test

@WofWca
Copy link
Collaborator Author

WofWca commented Oct 29, 2024

Let me rebase

There are a lot of params, it's getting hard not to mix them up.
And I want to add another one.

I have dobule-checked that this commit doesn't mix up the arguments.
Follow-up to the previous commit.
Add `scrollIntoViewArg` parameter to `jumpToMessage`
and specify different values for it depending on context.

More specifically, prefer scrolling the target message
to the center, e.g. when showing the first unread message.
Prior to this commit, we'd scroll the first unread message
such that it's at the bottom of the message list,
which can be considered a regression, which was introduced in
5f0efe1.

Closes #4284.
@WofWca WofWca force-pushed the wofwca/fix-scroll-to-first-unread branch from 4fd1d57 to 22aeadf Compare October 29, 2024 15:34
@WofWca WofWca merged commit c28d796 into main Oct 29, 2024
6 of 7 checks passed
@WofWca WofWca deleted the wofwca/fix-scroll-to-first-unread branch October 29, 2024 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/ux UI/UX related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fresh messages: opening a chat only shows the first one, you need to scroll for the rest
3 participants