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
Add support for thread view not rendering parent message #2636
Add support for thread view not rendering parent message #2636
Conversation
Kudos, SonarCloud Quality Gate passed! |
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.
LGTM 🔥
added a small comment but nothing blocking for sure.
@@ -106,6 +106,34 @@ final class ChatThreadVC_Tests: XCTestCase { | |||
) | |||
} | |||
|
|||
func test_whenThreadRendersParentMessageEnabledIsFalse() { |
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.
nit: Shall we add something as the expected result? Something like
test_whenThreadRendersParentMessageEnabledIsFalse_parentMessageWontRender
?
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.
I've opted to follow the same pattern in the rest of the tests in ChatThreadVC_Tests, for example: test_whenShouldMessagesStartAtTheTopIsTrue()
When the expected result is too obvious, I don't mind omitting it, but I don't mind being explicit, either. I think, in the end, it is just personal taste.
In this case, because it is consistent with the rest of the tests in this class, I'm willing not to change it, but let me know if you have a different opinion 👍
🔗 Issue Links
Resolves https://github.com/GetStream/ios-issues-tracking/issues/413
🎯 Goal
Adds a new flag to disable rendering the root message in the Thread View.
📝 Summary
Components.threadRendersParentMessageEnabled
flag☑️ Contributor Checklist