migrated account fragment to jetpack compose #1561
Conversation
NiranjanNlc
force-pushed
the
accountFragmentToCompose
branch
2 times, most recently
from
3de6f3e to
450baa7
Compare
|
Hi @NiranjanNlc I think this UI is a bit different from the existing one . I would insist that you follow the original design |
| mRvLinkedBankAccounts!!.addOnItemTouchListener( | ||
| RecyclerItemClickListener( | ||
| activity, | ||
| object : RecyclerItemClickListener.OnItemClickListener { | ||
| override fun onItemClick(childView: View?, position: Int) { | ||
| val intent = Intent( | ||
| activity, | ||
| BankAccountDetailActivity::class.java | ||
| ) | ||
| intent.putExtra( | ||
| Constants.BANK_ACCOUNT_DETAILS, | ||
| mBankAccountsAdapter!!.getBankDetails(position) | ||
| ) | ||
| intent.putExtra(Constants.INDEX, position) | ||
| startActivityForResult(intent, BANK_ACCOUNT_DETAILS_REQUEST_CODE) | ||
| } |
There was a problem hiding this comment.
and have you handled this case in the new compose UI ?
There was a problem hiding this comment.
oops i missed this , would implement it on next commit along with your comment on desighn and binding .
Thanks ....
| @JvmField | ||
| @BindView(R.id.tv_empty_no_transaction_history_subtitle) | ||
| var tvTransactionsStateSubtitle: TextView? = null | ||
| lateinit var binding: FragmentAccountsBinding |
There was a problem hiding this comment.
can we get rid of binding ? with compose we shouldn't need this
There was a problem hiding this comment.
it shall go while all the component of FinanceFragment is converted into jetpack compose.
Its because of relative layout on the previous one and can you provide me some hint on how should i approach for the relative layout in jetpack compose . |
NiranjanNlc
force-pushed
the
accountFragmentToCompose
branch
from
450baa7 to
7ffd760
Compare
Changed the desighn and implemented the onClickListener as you suggested . Please get it reviewed again and provide me suggestion. |
|
@PratyushSingh07 , |
| import org.mifos.mobilewallet.mifospay.bank.viewmodel.BankAccountsViewModel | ||
|
|
||
| @Composable | ||
| fun AccountScreen(viewModel: BankAccountsViewModel, |
There was a problem hiding this comment.
Code not formatted, Please format the code.
There was a problem hiding this comment.
done by ctrl+alt+L
| } | ||
|
|
||
| @Composable | ||
| fun AccountDisplay( |
There was a problem hiding this comment.
Rename it with AccountScreenContent
|
|
||
| @Preview | ||
| @Composable | ||
| fun AccountScreenPreview() { |
There was a problem hiding this comment.
Empty bank details preview is missing
There was a problem hiding this comment.
created the new preview function for empty list
| ) | ||
| ) | ||
| AccountDisplay(bankAccountDetails, {},{}) | ||
| // AccountDisplay(bankAccountDetails = emptyList(),{}) |
|
|
||
| @Preview | ||
| @Composable | ||
| fun AddAccountChipPreview() { |
There was a problem hiding this comment.
wrap around the MifosTheme every preview function
| import org.mifos.mobilewallet.mifospay.R | ||
|
|
||
| @Composable | ||
| fun ItemCausualList( |
There was a problem hiding this comment.
ambiguous naming convention. what is Causual
There was a problem hiding this comment.
Write something related to feature
There was a problem hiding this comment.
changed ListItemWithImage ..done
|
|
||
| @Preview | ||
| @Composable | ||
| fun ComposeItemPreview() { |
There was a problem hiding this comment.
Preview name should be function name + Preview - suffix
| import androidx.compose.ui.unit.dp | ||
| import org.mifos.mobilewallet.mifospay.R | ||
| @Composable | ||
| fun PlaceholderStateLayout( |
There was a problem hiding this comment.
changed to EmptyStatePlaceHolder
| binding= FragmentAccountsBinding.inflate(inflater, container, false) | ||
| ButterKnife.bind(this,binding.root) | ||
| binding.accountScreen.setContent { | ||
| AccountScreen(viewModel, ::addAccountClicked, ::onAccountClicked) |
There was a problem hiding this comment.
Don't need to pass the viewmodel init the ViewModel through hilt.
See CardsScreen as an example.
NiranjanNlc
force-pushed
the
accountFragmentToCompose
branch
from
7ffd760 to
7a59aec
Compare
|
closing this as completed by #1579 |
Issue Fix
Fixes #1533
Screenshots
Screenrecorder-2024-03-13-14-14-17-935.mp4
Description
Migrated AccountFragments to jetpack compose screen
Apply the
AndroidStyle.xmlstyle template to your code in Android Studio.Run the unit tests with
./gradlew checkto make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.