-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
ScrollView jumping improvements #2315
Conversation
This PR already fixed #2312 by disallowing first responder when a the chatvc is presenting another vc, which is the conflict. |
thanks a lot for diving into all that! i already run the draft in its current state, and i have to say, that it improves things significantly already. wow! please let us know, when it is ready-to-review - or if you have any questions
i am not sure what you mean here - i just tested receiving messages while using the input field - and that works as expected
this is totally fine, and also how it was intended to work. this is also how it works on android/desktop: if the chat is not "full", messages are at the bottom, close to the input field |
I mean while selecting messages (long press message > more options)
OK that's easy then. My reference (iMessage) keeps it at the top so I thought that was intended.
It is basically all I wanted to do for this PR, I opened it in draft because I wasn't sure if it was OK to create PRs of this size. |
This PR RPReplay_Final1728752673.movRelease version RPReplay_Final1728752673.mov |
The image from the read me showing the chat screen is no longer accurate since this PR (since messages float down instead of up here) |
af65d6f broke first message scrolling while trying to prevent the empty state view from jumping up and down when keyboard appears. |
Fixed twitching by adding it to the background view instead of to the actual tableView content
Resolved |
RPReplay_Final1728837824.mp4 |
@r10s I don't have permissions to add reviewers |
This is similar to how KDeltaChat does it, having scrollview laying out its elements from bottom to top is easier than trying to scroll to bottom on new messages. |
I am amazed thanks a lot!!! 🤩 |
Wait then who am I? |
In this PR I flipped the scrollView so now new messages automatically cause the rest of the messages to move up so we no longer need to programmatically scroll down when messages are sent and/or received.
Fixes:
Intended changes:
Side effects:
Messages are not refreshed until user stops editing
This is because we rely on selectedIndexPaths to keep which messages are selected, which can get out of sync with the actual message list when new messages are received or when they are automatically deleted. This was already the case before with deleted messages (eg through auto deletion) but is now also the case for new messages because messages are now pushed at the beginning of the indexPaths instead of at the end (because tableView is flipped).
This can be mitigated by tracking selected messages instead of relying on selectedIndexPaths.
Messages don't float to the top when you have only few messages
Since tableView now starts at the bottom, when you only have like 2 messages they are at the bottom instead of at the top.
This can totally be mitigated with a bit more effort.
Empty state view is a bit higher up
I think this is due to safeArea change to tableView but it is not a bad change because now it is still visible when keyboard is up (on iPhone SE anyway)
Notes:
I feel like it would be better if we did not use inputAccessoryViews that switch between keyboard and view controller attachment but instead just a normal view which we animate to always stay above the keyboard. This would eliminate the need for much of the first responder juggling we do right now and also eliminate the edge case where the input view is hidden when you don't expect it.
Would be really nice to have tests that can confirm if the keyboard comes back after actions when expected.