-
Notifications
You must be signed in to change notification settings - Fork 110
Fix message thread reply footnote view not shown if parent message not in cache #681
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
Fix message thread reply footnote view not shown if parent message not in cache #681
Conversation
…ter and do workaround" This reverts commit cfb4b2e.
…to be fetched This solution requires the least amount of changes and there is also no breaking change
SDK Size
|
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.
Great idea and solution, it's the best we can do atm ✅ Left one suggestion, let me know what you think.
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageContainerView.swift
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageRepliesView.swift
Outdated
Show resolved
Hide resolved
Sources/StreamChatSwiftUI/ChatChannel/MessageList/MessageRepliesView.swift
Show resolved
Hide resolved
self.factory = factory | ||
self.channel = channel | ||
self.message = message | ||
parentMessageObserver.controller.synchronize() |
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.
shouldn't we do this only onAppear
? init
can be called many times.
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.
Xcode 15 was not happy with the current implementation, so I moved to onAppear, but this requires changing from Group, to use a VStack
|
🔗 Issue Link
Fixes https://linear.app/stream/issue/IOS-559/thread-reply-footnote-view-not-shown-in-reply-show-in-channel
🎯 Goal
Fixes messages thread reply footnote view not shown when the parent message is not in cache.
🛠 Implementation
Problem
The issue is that in order to render the
MessageRepliesView
, we require aparentMessage
. We get theparentMessage
from thedataStore
, but if the parent message is from an old page, it will returnnil
since it won't be in the local DB.We can't change the
parentMessage
parameter to anoptional
since that would be a breaking change, and changing the Factory protocol could even lead to a silent breaking change for our customers.Solution
1st try: Breaking Change
This one is tricky to solve without breaking changes. First, I tried to force a breaking change by making the function in the Factory @unavailable. But this is not possible. It is only supported by @objc protocols. Since we have default implementations of the factory, creating a real breaking change would be silent for customers, so this is not an option.
2nd try: Hack-ish Workaround
The first solution I did without breaking change was to provide the message reply as the parent message as well. Then, check if they had the same ID, and treat it as if parentMessage was nil. You can see the implementation here.
The problem with this approach is that it is very hacky and unexpected. I customer would need to know about this hack if they wanted to customize the view.
3rd try: Final Solution
The solution I found the best was to create a Lazy view of the existing
MessageRepliesView
. Whenever the parent message does not exist in the local cache, we fetch it and then display the view. From here on, it will be in the local cache until we logout, so it won't be always fetching.This approach requires the least changes on our side and no changes on the customer side. The con, is whenever this case happens, it means additional requests will be made just to display a footnote view. Ideally, this request would only be executed only when opening the Thread View, but unfortunatly, until we have v5, there's no alternative.
🧪 Testing
🎨 Changes
Below is the fix demo. The thread reply view takes a bit of time to load since it is lazy-loaded and requires performing a request to show it.
Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-05.at.18.52.50.mp4
☑️ Checklist