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

Reply preview improvements and reply preview click smooth scroll #151

Merged
merged 7 commits into from
Sep 22, 2024

Conversation

fmzbl
Copy link
Contributor

@fmzbl fmzbl commented Sep 18, 2024

makes clicking a reply preview trigger a smooth scroll to the item, which is then highlighted.

@fmzbl fmzbl force-pushed the reply-preview-improvements branch from 86fc350 to 882b611 Compare September 19, 2024 10:45
@fmzbl fmzbl marked this pull request as ready for review September 19, 2024 10:47
@fmzbl fmzbl force-pushed the reply-preview-improvements branch from 6f8f2dc to 882b611 Compare September 19, 2024 11:10
@fmzbl fmzbl requested a review from kevinaboos September 19, 2024 11:14
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Looks great! Just left one comment/question for you.

src/home/room_screen.rs Outdated Show resolved Hide resolved
kevinaboos
kevinaboos previously approved these changes Sep 22, 2024
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Great work, thanks!

src/home/room_screen.rs Outdated Show resolved Hide resolved
which we now use for scrolling up to replies.

Highlight a replied-to message upon click (after auto-scroll to it)
in a light blue color, which makes it much more obvious.

Explicitly specify the default font color of Html widgets.
that have not yet been fetched in the timeline
Use `Vector::focus()` with reverse iteration from the current reply
message, searching backwards since the replied-to message must come
before the reply message
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. I made some changes to how we search for the replied-to message in the timeline, mainly just to make it a lot faster.

I found quite a few scenarios where the smooth_scroll_to() function does not work as expected. Most of them occur when the message is already visible, but at least the highlighting makes it very obvious where the message is. Sometimes it just straight up doesn't work, but since it sounds like you're already working on improving the smooth_scroll_to() function, we can go ahead and merge this and then add future improvements later.

@kevinaboos kevinaboos merged commit bf82072 into project-robius:main Sep 22, 2024
1 check passed
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.

2 participants