-
Notifications
You must be signed in to change notification settings - Fork 687
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
Keep chat stickied to the bottom on resize #346
Conversation
return this.unbind(".sticky"); | ||
}; | ||
|
||
$.fn.sticky = function() { | ||
var self = this; | ||
var stuckToBottom = true; | ||
var windowResizeHandler = function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename it to something more generic and make "msg.sticky"
event use this callback too (it's same code).
65e51b9
to
7bcac1b
Compare
@xPaw Do you like that version better? |
Sure. Need to test it now. |
It works decently well for its intended use. I tried to make it a bit more solid when doing weird violent horizontal resizing, but it still manages to unstick once in a while. Probably a race condition between |
Just checking (I haven't tested yet): if scrolling back, then switching to horizontal mode, do we properly stay where we were, instead of being sent to the bottom? |
@astorije: yes, it only restick to bottom if it already was. If you scroll up it doesn't do anything (same as when receiving a message in the channel which uses the same function). |
I've tested this, and it does work. As you already noted, horizontal resizing causes it to unstick. I'm not sure if there's any easy solution to this problem, but maybe try looking at it one more time? I'm 👍 with this PR, even with horizontal bugginess. |
Fixes the chat not staying at the bottom when opening the on-screen keyboard on mobile.
@xPaw: Fixed it. I'm not a fan of using time like that, but since the issue is effectively an async timing issue, that's the only solution. It still will unstick if the browser lags for more than 250ms during the resizing. According to some logging I did, it's very, very unpredictable. 250ms should cover most cases, but it's still possible it unsticks. I also tried with timers and other things, but in the end it doesn't change that we have a race condition between the time the |
.on("scrollBottom.sticky", function() { | ||
stuckToBottom = true; | ||
lastStick = Date.now(); | ||
this.scrollTop = this.scrollHeight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This variable is never used
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What variable? this
in this context is the element, so this scrolls to the bottom. Would be silly to wrap this
in jQuery to end up doing the exact same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EDIT: I am an idiot.
Keep chat stickied to the bottom on resize
Fixes the chat not staying at the bottom when opening the on-screen keyboard on mobile.