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

Draft: improve scrolling behavior #1767

Closed
wants to merge 7 commits into from
Closed

Conversation

cyBerta
Copy link
Member

@cyBerta cyBerta commented Dec 8, 2022

Trying to figure out the root cause for the weird scroll behavior. This PR is only to discuss possible changes.
The reason I started investigating this is #1691. The fix for that issue is fairly simple, see af5cefe, if we take the DC Android as an template. However the weird scrolling down and back up when sending a message always happens now. I will explain a little bit more in the comments.

Generally I've noticed that the scrolling-down and up happens, because the messageInputBar looses it's focus when the user taps on the send button. As a result normally the keyboard would disappear. We mitigate that by re-requesting the focus for the messageInputBar when the messageInputBar's UITextViewDelegate calls back textViewShouldEndEditing(). Normally that is sufficient to keep the keyboard in place, on some devices and OS versions you may see a very small flickering.
However the frame size of the UITableView immediately changes and all cells at the bottom get moved for a short moment to the bottom of the screen, before the size frame size regains it's previous size - above the keyboard.

While experimenting and reading on stack overflow for possible solutions I learned that we can use contentInset to avoid the content is dragged down to the bottom of the screen. I vaguely remember we already worked with contentInsets in the past in this controller, but scratched that approach because of other issues we had with it like wrong initialization.

My current idea is - contrary to what we tried in the past regarding contentInsets - to set the content inset only temporarily at the bottom before the keyboard events are thrown / the tableViewFrame changes and reset it as soon as the reload() was done and the new message bubble appeared (but is still hidden below the keyboard).

What's not yet clear to me is if any call to scrollDown would be still necessary after resetting the contentInset, or maybe after animating the contentInset value.

What I can confirm so far is that setting the contentInset in textViewShouldEndEditing (= before the keyboard events are thrown / before the tableViewFrame size changes) indeed prevents the messages being moved down if I comment out scrolling completely.

@cyBerta
Copy link
Member Author

cyBerta commented Dec 21, 2022

closes also #1691

@cyBerta cyBerta added the message-list bugs related to the message list label Jan 16, 2023
@r10s
Copy link
Member

r10s commented Jan 17, 2023

ftr: we decided to tackle this together with the other message-list bugs related to the message list issues beginning of february.

@r10s r10s marked this pull request as draft February 15, 2023 12:40
@r10s
Copy link
Member

r10s commented Feb 15, 2023

after playing around a little, this pr does not really fix the message-list issued, it maybe mitigates them, but there is still:

  • jumping up/down
  • input bar laying over bubbles sometimes (this is easily reproducible by entering a chat, swiping a little up)
  • keyboard does not always appear

we will probably go for a bit larger rewrite in #1810 therefore.

@r10s
Copy link
Member

r10s commented Oct 5, 2023

closing stale draft, superseded by #1810

@r10s r10s closed this Oct 5, 2023
@r10s r10s added this to Closed PRs and Issues in Project Resurrection via automation Oct 5, 2023
@r10s
Copy link
Member

r10s commented Oct 29, 2023

cmp #1737

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
message-list bugs related to the message list
Projects
Project Resurrection
Closed PRs and Issues
Development

Successfully merging this pull request may close these issues.

None yet

2 participants