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

perf: hide/show on-hover shortcut menu with CSS #3751

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

WofWca
Copy link
Collaborator

@WofWca WofWca commented Apr 5, 2024

Why WIP:

...instead of JS. Previously hovering over a different message would
cause the whole message list to re-render.

There are slight behavioral changes:

  • Previously if you pointed the cursor to a message, then scrolled the
    chat without moving the pointer, it would keep the menu on top
    of the initial message, despite the cursor pointing at a different
    message after the scroll.
  • Now when the three-dot menu is clicked, the shortcuts menu disappears
  • When you point on emojis, the menu also disappears

The new behavior more closely resembles that of Signal.

@WofWca WofWca force-pushed the perf-shortcut-menu-css branch 2 times, most recently from 3046199 to 59488f7 Compare April 5, 2024 11:50
@WofWca WofWca requested a review from adzialocha April 5, 2024 11:50
Copy link
Member

@adzialocha adzialocha left a comment

Choose a reason for hiding this comment

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

Thank you, using plain CSS for this is definitely better like that.

The new approach is a little bit more visually "active" but since the elements are so discreet one almost doesn't notice it.

Comment on lines +67 to 70
<li id={props.key2} className='message-wrapper'>
<Message {...props} />
<div className='message-observer-bottom' id={'bottom-' + props.key2} />
</li>
Copy link
Member

Choose a reason for hiding this comment

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

We could use the new SCSS module approach here for anything new (styles in scss folder are deprecated)

Copy link
Member

@Simon-Laux Simon-Laux Apr 10, 2024

Choose a reason for hiding this comment

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

we can move it later along with the other remaining message styles.

@Simon-Laux
Copy link
Member

please rebase onto master

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.

Thanks, this improves scroll performance a lot for me.

...instead of JS. Previously hovering over a different message would
cause the whole message list to re-render.

There are slight behavioral changes:
- Previously if you pointed the cursor to a message, then scrolled the
    chat without moving the pointer, it would keep the menu on top
    of the initial message, despite the cursor pointing at a different
    message after the scroll.
- Now when the three-dot menu is clicked, the shortcuts menu disappears
- When you point on emojis, the menu also disappears

The new behavior more closely resembles that of Signal.
@WofWca WofWca marked this pull request as ready for review April 9, 2024 14:23
@Simon-Laux Simon-Laux merged commit 6213828 into deltachat:master Apr 10, 2024
6 checks 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.

3 participants