-
Notifications
You must be signed in to change notification settings - Fork 28
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
refactor: unify and simplify conversation lists [WPB-9433] #3480
refactor: unify and simplify conversation lists [WPB-9433] #3480
Conversation
…tor/unify-and-simplify-conversation-lists
Ups 🫰🟨This PR is too big. Please try to break it up into smaller PRs. |
…tor/unify-and-simplify-conversation-lists
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.
Looks good to me! Before adding the ✅ What about adding ADR? since we are refactoring and removing lots of code (👏 ) and unifying to new ones (🙌 )
New ADR(s) in this PR 📚:5. Conversation list composables refactorDate: 2024-10-01 StatusAccepted ContextConversation-list-related composables and screens (all conversations, archive) are overly DecisionSimplify and unify composables related to creating screens that show conversation lists. Consequences
|
Good idea! I forgot about it, let me know if the ADR that I just added to this PR is fine or needs some improvements. 😄 |
Quality Gate passedIssues Measures |
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.
🚀
Built wire-android-staging-compat-pr-3480.apk is available for download |
Built wire-android-dev-debug-pr-3480.apk is available for download |
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Issues
Conversation list composables are overly complicated and have many leftovers after bottom tabs and multiple refactors, so in order to implement pagination, it's better to first make a cleanup.
Solutions
ConversationsScreenContent
is a composable used to create screens with conversation listsConversationItemType
fromConversationRouterState
, extracted it into dedicated file and renamed toConversationsDialogsState
as from now on it only contains conversation-list-related dialogs statesconversationSearchResult
andfoldersWithConversations
into single one as having two separate fields is not necessary and actually would complicate it even more when implementing paginationConversationListViewModel
to useAssistedInject
and thanks to that reduced the number of functions andLaunchedEffects
neededConversationListViewModel
andConversationCallListViewModel
and “Preview” versions of them to be able to create previews for screens that show conversation list and useConversationsScreenContent
Activity
toContext
for functions that return call intents, to be able to show previews (there is noActivity
for composable previews)Arrangement
classesTesting
Test Coverage (Optional)
How to Test
Open all conversations list or archive list and make sure everything works as it should.
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764
.