-
Notifications
You must be signed in to change notification settings - Fork 468
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: profile fragment migrated to compose #1547
Conversation
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.
Please review this @therajanmaurya
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/ui/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/ui/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/presenter/ProfileViewModel.kt
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/presenter/ProfileViewModel.kt
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/ui/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/ui/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
787d7e5
to
253c204
Compare
private val mPreferencesHelper: PreferencesHelper | ||
) : ViewModel() { | ||
|
||
private val _bitmapImage = MutableStateFlow<Bitmap?>(null) |
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.
Create ProfileUiState sealed class and move these fields into that.
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.
if (bitmap == null) { | ||
MifosTextUserImage( | ||
modifier = modifier, | ||
text = username?.firstOrNull()?.toString() ?: "J" |
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.
Here image should have 3 condition
- If bitmap is not null then load bitmap
- If bitmap is null then check username or name and set first char
- As bitmap is null and we set username or name first char then we need to set plus circular image at bottom right of the image that allows user to open update user profile photo.
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.
The 3rd condition can be handled after the user clicks on the Edit Profile
button @therajanmaurya . Hence Adding a plus icon in my opinion isn't the best alternative
...ystem/src/main/java/org/mifos/mobilewallet/mifospay/designsystem/component/MifosUserImage.kt
Outdated
Show resolved
Hide resolved
...c/main/java/org/mifos/mobilewallet/mifospay/designsystem/component/ProfileConcentricImage.kt
Outdated
Show resolved
Hide resolved
...m/src/main/java/org/mifos/mobilewallet/mifospay/designsystem/component/MifosTextUserImage.kt
Outdated
Show resolved
Hide resolved
mifospay/src/main/java/org/mifos/mobilewallet/mifospay/home/ui/ProfileScreen.kt
Outdated
Show resolved
Hide resolved
import org.mifos.mobilewallet.mifospay.ui.utility.Orientation | ||
|
||
@Composable | ||
fun ProfileItemCard( |
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.
Explore the Flow https://developer.android.com/jetpack/compose/layouts/flow#item-weights this Horizontal and Vertical thing can be handled directly through the single below Flow. See example in above Flow documentation.
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.
Wont the FlowRow cause inconsistent UI for different screen sizes? In case of screens with smaller width the elements will flow out.
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.
No, Flow is built for that specially.
import org.mifos.mobilewallet.mifospay.ui.utility.Orientation | ||
|
||
@Composable | ||
fun ProfileItemCard( |
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.
No, Flow is built for that specially.
0b66c4f
to
ca4ac61
Compare
Issue Fix
Fixes #1532
figma.profile.mp4
Apply the
AndroidStyle.xml
style template to your code in Android Studio.Run the unit tests with
./gradlew check
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.