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

chore: refactor scroll to bottom logic #11872

Merged
merged 5 commits into from
Mar 22, 2024

Conversation

DorraJaouad
Copy link
Contributor

☑️ Resolves

  • Refactor scrolling to bottom logic that is used when:
    • Reaction added to the last messages
    • Polling new messages
    • Uploading file
    • Scroll to bottom button

🚧 TODO follow-up :

  • Adjust placeholder height to meet clientHeight (it scrolls when joining a conversation) !

@DorraJaouad DorraJaouad added this to the 💞 Next Beta (29) milestone Mar 20, 2024
@DorraJaouad DorraJaouad self-assigned this Mar 20, 2024
@nickvergessen
Copy link
Member

Does it still scroll if I have the chat open in a call on my second screen and work in my IDE on the other screen?

src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
@Antreesy
Copy link
Contributor

Antreesy commented Mar 20, 2024

Filling the truth table for 5 arguments we're using:

🛑 - no scroll at all
❎ - half scroll, user haven't read recent messages
🔽 - to the bottom, user is reading everything
bg - background, tab not visible
fg - foreground, tab visible (on a second screen)
ATB - chat is already at the bottom
⚠️ - was changed with that PR
🚀 - nice to have

Remark: Before has only these rules applied, so behavior might differ a bit:

  • force: true - 🔽 upload file only
  • isSticky - ATB + messages loaded (not new chat) - 🔽 new messages, new reactions
  • isVisible && (hasFocus || isInCall) - 🔽 click to bottom, posting , otherwise ❎ half scroll

Remark: !isVisible && hasFocus is impossible case, omitted from table

visible focus InCall o.force sticky before after goal case diff
0 0 0 0 0 🛑 🛑 🛑 left tab on bg
0 0 0 0 1 left tab ATB
0 0 0 1 0 🔽 🔽 upload and left ⚠️
0 0 0 1 1 🔽 🔽 upload and left ATB ⚠️
0 0 1 0 0 🛑 🛑 🛑 left call on bg
0 0 1 0 1 left call ATB
0 0 1 1 0 🔽 🔽 upload at call bg ⚠️
0 0 1 1 1 🔽 🔽 upload at call bg ATB ⚠️
1 0 0 0 0 🛑 🛑 🛑 chat on fg
1 0 0 0 1 🔽 chat fg, ATB, passive reading 🚀
1 0 0 1 0 🔽 🔽 upload and blur ⚠️
1 0 0 1 1 🔽 🔽 upload ATB and blur ⚠️
1 0 1 0 0 🛑 🛑 🛑 call on fg
1 0 1 0 1 🔽 🔽 🔽 call, passive reading
1 0 1 1 0 🔽 🔽 🔽 upload at call and blur
1 0 1 1 1 🔽 🔽 🔽 upload at call ATB and blur
1 1 0 0 0 🛑 🛑 🛑 reading old messages
1 1 0 0 1 🔽 🔽 🔽 chatting
1 1 0 1 0 🔽 🔽 🔽 upload file
1 1 0 1 1 🔽 🔽 🔽 upload + chat
1 1 1 0 0 🛑 🛑 🛑 reading old chat in call
1 1 1 0 1 🔽 🔽 🔽 chatting in call
1 1 1 1 0 🔽 🔽 🔽 upload in call
1 1 1 1 1 🔽 🔽 🔽 upload + chat in call

if isWindowVisible is falsy, it drops all other cases for you, and when it comes to second if block, you don't have to check for visible/focus/sticky
force should be truthy if you posting a message/file, or at the chat opening, to scroll to the focused / first unread

Looking at the cycle, isInCall and hasFocus can be omitted, and 'force' treated separately before other logic:

if (force) {
    🔽 
} else if (!sticky) {
    🛑 
} else if (!visible) {
     
} else {
    🔽
}

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Algorithm looks fine. Left some comments.

One more thing: loaded chat opens on top, there should be a point or watcher to look after it and set scroll position directly

src/components/NewMessage/NewMessage.vue Show resolved Hide resolved
src/components/MessagesList/MessagesList.vue Outdated Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the chore/noid/refactor-chat-scrolling branch from 9beca0c to 7cbc5d3 Compare March 22, 2024 12:46
@DorraJaouad DorraJaouad merged commit 35beec6 into main Mar 22, 2024
46 checks passed
@DorraJaouad DorraJaouad deleted the chore/noid/refactor-chat-scrolling branch March 22, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants