-
Notifications
You must be signed in to change notification settings - Fork 26
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
feat: show connecting indicator if user has bad/lost connection during a call (WPB-1125) #2101
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2101 +/- ##
=============================================
+ Coverage 40.58% 40.59% +0.01%
Complexity 975 975
=============================================
Files 309 309
Lines 11367 11369 +2
Branches 1519 1519
=============================================
+ Hits 4613 4615 +2
Misses 6315 6315
Partials 439 439
Continue to review full report in Codecov by Sentry.
|
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1092 succeeded. The build produced the following APK's: |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1099 succeeded. The build produced the following APK's: |
Build 1099 failed. |
Build 1104 failed. |
Build 1110 succeeded. The build produced the following APK's: |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1112 succeeded. The build produced the following APK's: |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1122 succeeded. The build produced the following APK's: |
Build 1122 failed. |
Build 1125 succeeded. The build produced the following APK's: |
app/src/main/kotlin/com/wire/android/ui/calling/ongoing/participantsview/ParticipantTile.kt
Show resolved
Hide resolved
app/src/main/kotlin/com/wire/android/ui/calling/ongoing/participantsview/ParticipantTile.kt
Outdated
Show resolved
Hide resolved
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1169 succeeded. The build produced the following APK's: |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1170 succeeded. The build produced the following APK's: |
APKs built during tests are available here. Scroll down to Artifacts! |
APKs built during tests are available here. Scroll down to Artifacts! |
Build 1175 succeeded. The build produced the following APK's: |
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.
Nicelly done ✅ Just one comment about tests
@@ -39,6 +39,7 @@ class UICallParticipantMapper @Inject constructor( | |||
isCameraOn = participant.isCameraOn, | |||
isSharingScreen = participant.isSharingScreen, | |||
avatar = participant.avatarAssetId?.let { ImageAsset.UserAvatarAsset(wireSessionImageLoader, it) }, | |||
membership = userTypeMapper.toMembership(participant.userType) | |||
membership = userTypeMapper.toMembership(participant.userType), | |||
hasEstablishedAudio = participant.hasEstablishedAudio |
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.
Can we write some test for it?
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.
it's already tested in UICallParticipantMapperTest.kt
…ing_label_in_call # Conflicts: # app/src/main/res/values/strings.xml
Build 1238 succeeded. The build produced the following APK's: |
@@ -286,6 +288,7 @@ private fun UsernameTile( | |||
hasEstablishedAudio: Boolean | |||
) { | |||
val color = if (isSpeaking) MaterialTheme.wireColorScheme.primary else Color.Black | |||
val nameLabelColor = if (hasEstablishedAudio) Color.White else colorsScheme().secondaryText |
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.
thought: maybe we should move all these hard-coded to the theme colors as well, they would just be the same for both light and dark mode, but it would ensure we have all colors in one place so if we decide to change one of them we wouldn't need to look for them in the whole project
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.
Well, not that simple by just changing the color in the file.. we need to check the whole project in all cases I would say especially for white and black
PR Submission Checklist for internal contributors
The PR Title
SQPIT-764
The PR Description
What's new in this PR?
Description
This PR aims to display
Connecting...
indicator in the participant tile to indicate that the user has a bad/lost network durning a call. We also grey out the tile in that network state.Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
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
.