Skip to content
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

Fix: Avatar is not visible when the profile is disabled but the Avatar is set to public #568

Merged
merged 3 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,7 @@ internal fun AvatarPicker(uiState: AvatarPickerUiState, onEvent: (AvatarPickerEv
}
ProfileCard(
profile = uiState.profile,
email = uiState.email,
avatarCacheBuster = uiState.avatarCacheBuster.toString(),
modifier = Modifier.padding(horizontal = 16.dp),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,21 @@ import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.dp
import com.gravatar.AvatarQueryOptions
import com.gravatar.DefaultAvatarOption
import com.gravatar.ImageRating
import com.gravatar.extensions.avatarUrl
import com.gravatar.extensions.defaultProfile
import com.gravatar.restapi.models.Profile
import com.gravatar.types.Email
import com.gravatar.ui.GravatarTheme
import com.gravatar.ui.components.ComponentState
import com.gravatar.ui.components.ProfileSummary
import com.gravatar.ui.components.atomic.Avatar
import com.gravatar.ui.components.atomic.ViewProfileButton
import com.gravatar.ui.components.transform

@Composable
internal fun ProfileCard(
profile: ComponentState<Profile>?,
email: Email,
modifier: Modifier = Modifier,
avatarCacheBuster: String? = null,
) {
Expand All @@ -49,16 +50,15 @@ internal fun ProfileCard(
avatar = {
val sizePx = with(LocalDensity.current) { 72.dp.roundToPx() }
Avatar(
state = profile.transform {
avatarUrl(
AvatarQueryOptions {
preferredSize = sizePx
rating = ImageRating.X
},
).url(avatarCacheBuster).toString()
email = email,
avatarQueryOptions = AvatarQueryOptions {
preferredSize = sizePx
rating = ImageRating.X
defaultAvatarOption = DefaultAvatarOption.Status404
},
size = 72.dp,
modifier = Modifier.clip(CircleShape),
cacheBuster = avatarCacheBuster,
)
},
viewProfile = { state ->
Expand Down Expand Up @@ -106,6 +106,7 @@ private fun ProfileCardPreview() {
profile = ComponentState.Loaded(
defaultProfile(hash = "dfadf", "John Travolta"),
),
email = Email("[email protected]"),
modifier = Modifier.padding(20.dp),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,9 @@ internal fun OauthPage(
uiState.profile?.let {
ProfileCard(
profile = it,
email = email,
modifier = Modifier.padding(top = 16.dp),
avatarCacheBuster = uiState.avatarCacheBuster,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably don't have to use the cache buster on this page, but:

  • it makes testing easier, and
  • it makes sure the the QE always shows the latest and correct avatar

)
}
val sectionModifier = Modifier.padding(top = 24.dp, bottom = 10.dp)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.gravatar.ui.components.ComponentState
internal data class OAuthUiState(
val status: OAuthStatus = OAuthStatus.LoginRequired,
val profile: ComponentState<Profile>? = null,
val avatarCacheBuster: String? = null,
)

internal sealed class OAuthStatus {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import androidx.lifecycle.viewmodel.CreationExtras
import com.gravatar.quickeditor.QuickEditorContainer
import com.gravatar.quickeditor.data.storage.ProfileStorage
import com.gravatar.quickeditor.data.storage.TokenStorage
import com.gravatar.quickeditor.ui.time.Clock
import com.gravatar.quickeditor.ui.time.SystemClock
import com.gravatar.services.ErrorType
import com.gravatar.services.GravatarResult
import com.gravatar.services.ProfileService
Expand All @@ -28,8 +30,9 @@ internal class OAuthViewModel(
private val tokenStorage: TokenStorage,
private val profileStorage: ProfileStorage,
private val profileService: ProfileService,
clock: Clock,
) : ViewModel() {
private val _uiState = MutableStateFlow(OAuthUiState())
private val _uiState = MutableStateFlow(OAuthUiState(avatarCacheBuster = clock.getTimeMillis().toString()))
val uiState: StateFlow<OAuthUiState> = _uiState.asStateFlow()

private val _actions = Channel<OAuthAction>(Channel.BUFFERED)
Expand Down Expand Up @@ -127,6 +130,7 @@ internal class OAuthViewModelFactory(
tokenStorage = QuickEditorContainer.getInstance().tokenStorage,
profileStorage = QuickEditorContainer.getInstance().profileStorage,
profileService = QuickEditorContainer.getInstance().profileService,
clock = SystemClock(),
) as T
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import com.gravatar.extensions.defaultProfile
import com.gravatar.quickeditor.ui.gravatarScreenshotTest
import com.gravatar.types.Email
import com.gravatar.ui.components.ComponentState
import com.gravatar.uitestutils.RoborazziTest
import org.junit.Test
Expand All @@ -22,6 +23,7 @@ class ProfileCardTest : RoborazziTest() {
gravatarScreenshotTest {
ProfileCard(
profile = ComponentState.Loaded(profile),
email = Email("email"),
modifier = Modifier.padding(20.dp),
)
}
Expand All @@ -32,6 +34,7 @@ class ProfileCardTest : RoborazziTest() {
fun profileCardDarkMode() = gravatarScreenshotTest {
ProfileCard(
profile = ComponentState.Loaded(profile),
email = Email("email"),
modifier = Modifier.padding(20.dp),
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import app.cash.turbine.test
import com.gravatar.quickeditor.data.storage.ProfileStorage
import com.gravatar.quickeditor.data.storage.TokenStorage
import com.gravatar.quickeditor.ui.CoroutineTestRule
import com.gravatar.quickeditor.ui.time.Clock
import com.gravatar.restapi.models.Profile
import com.gravatar.services.ErrorType
import com.gravatar.services.GravatarResult
Expand All @@ -29,6 +30,7 @@ class OAuthViewModelTest {
private val tokenStorage = mockk<TokenStorage>()
private val profileService = mockk<ProfileService>()
private val profileStorage = mockk<ProfileStorage>()
private val clock = mockk<Clock>()
private val savedStateHandle = SavedStateHandle()

private lateinit var viewModel: OAuthViewModel
Expand All @@ -42,6 +44,7 @@ class OAuthViewModelTest {
coEvery { profileService.retrieveCatching(email) } returns GravatarResult.Success(mockk())
coEvery { profileStorage.getLoginIntroShown(any()) } returns false
coEvery { profileStorage.setLoginIntroShown(any()) } returns Unit
coEvery { clock.getTimeMillis() } returns 0
viewModel = createViewModel()
}

Expand Down Expand Up @@ -239,6 +242,6 @@ class OAuthViewModelTest {
}

private fun createViewModel(): OAuthViewModel {
return OAuthViewModel(savedStateHandle, email, tokenStorage, profileStorage, profileService)
return OAuthViewModel(savedStateHandle, email, tokenStorage, profileStorage, profileService, clock)
}
}
1 change: 1 addition & 0 deletions gravatar-ui/api/gravatar-ui.api
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ public final class com/gravatar/ui/components/atomic/AboutMeKt {

public final class com/gravatar/ui/components/atomic/AvatarKt {
public static final fun Avatar-EUb7tLY (Lcom/gravatar/restapi/models/Profile;FLandroidx/compose/ui/Modifier;Lcom/gravatar/AvatarQueryOptions;ZLandroidx/compose/runtime/Composer;II)V
public static final fun Avatar-EUb7tLY (Lcom/gravatar/types/Email;FLandroidx/compose/ui/Modifier;Lcom/gravatar/AvatarQueryOptions;Ljava/lang/String;Landroidx/compose/runtime/Composer;II)V
public static final fun Avatar-EUb7tLY (Lcom/gravatar/ui/components/ComponentState;FLandroidx/compose/ui/Modifier;Lcom/gravatar/AvatarQueryOptions;ZLandroidx/compose/runtime/Composer;II)V
public static final fun Avatar-uFdPcIQ (Lcom/gravatar/ui/components/ComponentState;FLandroidx/compose/ui/Modifier;Landroidx/compose/runtime/Composer;II)V
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,23 @@ import android.content.res.Configuration
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.size
import androidx.compose.runtime.Composable
import androidx.compose.runtime.getValue
import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import coil.compose.AsyncImage
import com.gravatar.AvatarQueryOptions
import com.gravatar.AvatarUrl
import com.gravatar.extensions.avatarUrl
import com.gravatar.extensions.defaultProfile
import com.gravatar.restapi.models.Profile
import com.gravatar.types.Email
import com.gravatar.ui.R
import com.gravatar.ui.components.ComponentState
import com.gravatar.ui.components.LoadingToLoadedProfileStatePreview
Expand Down Expand Up @@ -144,11 +151,70 @@ private fun EmptyAvatar(size: Dp, modifier: Modifier = Modifier) {
private fun Avatar(model: Any?, size: Dp, modifier: Modifier) {
AsyncImage(
model = model,
contentDescription = "User profile image",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was hardcoded in the component, we could probably skip the waiting for the translation for this one, as it will delay the release again.

contentDescription = stringResource(R.string.gravatar_ui_avatar_content_description),
modifier = modifier.size(size),
)
}

private enum class AvatarState {
None,
Loading,
Loaded,
Placeholder,
}

/**
* Atomic Avatar composable that displays a user's avatar that is generated from the user's email address.
* A skeleton overlay will be shown while loading the image.
*
* @param email The user's email address
* @param size The size of the avatar
* @param modifier Composable modifier
* @param avatarQueryOptions Options to customize the avatar query
* @param cacheBuster Random string value to force a cache bust
*/
@Composable
public fun Avatar(
email: Email,
size: Dp,
modifier: Modifier = Modifier,
avatarQueryOptions: AvatarQueryOptions? = null,
cacheBuster: String? = null,
) {
var state by remember { mutableStateOf(AvatarState.None) }
val sizePx = with(LocalDensity.current) { size.roundToPx() }
Box(
modifier = modifier.size(size),
) {
AsyncImage(
model = AvatarUrl(
hash = email.hash(),
avatarQueryOptions = AvatarQueryOptions {
preferredSize = sizePx
rating = avatarQueryOptions?.rating
forceDefaultAvatar = avatarQueryOptions?.forceDefaultAvatar
defaultAvatarOption = avatarQueryOptions?.defaultAvatarOption
},
).url(cacheBuster).toString(),
contentDescription = stringResource(R.string.gravatar_ui_avatar_content_description),
onLoading = {
state = AvatarState.Loading
},
onError = {
state = AvatarState.Placeholder
},
onSuccess = {
state = AvatarState.Loaded
},
)
when (state) {
AvatarState.Loading -> SkeletonAvatar(size = size)
AvatarState.Placeholder -> EmptyAvatar(size = size)
else -> Unit
}
}
}

@Preview
@Composable
private fun AvatarPreview() {
Expand Down
1 change: 1 addition & 0 deletions gravatar-ui/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@
<string name="gravatar_ui_empty_state_user_info">Add your location, pronouns, etc</string>
<string name="gravatar_ui_empty_state_view_profile_button">Claim profile</string>
<string name="gravatar_ui_location_empty_state">Location</string>
<string name="gravatar_ui_avatar_content_description">User profile image</string>
</resources>