Create MutedAccount SheetState #685
Conversation
Summary of ChangesHello @joelmuraguri, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request lays the groundwork for a 'Muted Accounts' feature by creating the necessary UI state management and presentation components. It defines the data models, actions, and a state holder using Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the state management structure for a new 'Muted Accounts' bottom sheet. The overall approach is sound and aligns with the project's architecture. However, the implementation is incomplete, with key business logic missing. My review provides feedback on implementing the missing logic and simplifying the state handling to prevent potential bugs.
| when (val action = type()) { | ||
| MutedAccountAction.Load -> { | ||
| TODO("Implement Load action") | ||
| } | ||
|
|
||
| is MutedAccountAction.Add -> { | ||
| TODO("Implement Add Account action") | ||
| } | ||
|
|
||
| is MutedAccountAction.Remove -> { | ||
| TODO("Implement Remove Account action") | ||
| } | ||
| } |
There was a problem hiding this comment.
The core logic for handling MutedAccountActions is missing and is currently stubbed with TODOs. Please implement the logic for loading, adding, and removing muted accounts to make this feature functional.
| scope: BottomSheetScope, | ||
| ) : BottomSheetState(scope) { | ||
|
|
||
| internal var currentAccounts by mutableStateOf<List<MutedAccount>>(emptyList()) |
There was a problem hiding this comment.
The currentAccounts property appears to be unused and duplicates state that should be managed by the MutedAccountStateHolder. To maintain a single source of truth, this property should be removed.
The list of muted accounts can be collected directly from the mutator's state flow within the MutedAccountsBottomSheet composable, for example:
@Composable
private fun MutedAccountsBottomSheet(state: MutedAccountsSheetState) {
val mutedAccountsState by state.mutator.state.collectAsStateWithLifecycle()
state.ModalBottomSheet {
// Use mutedAccountsState.mutedAccounts here
}
}
|
Let's put a pause on this. We still need to fully establish the pattern for these bottom sheets that have their own state holders. Lets finish building out the full muted words feature before adding more stubs. |
No description provided.