From 78de70881f71d623df7eed8c84a2a123516027af Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Fri, 11 Mar 2022 18:35:43 -0500 Subject: [PATCH] Fix responsiveness of profile photo edit UI. There were various issues around the profile photo updating correctly in the edit view. We want to make sure that what the user sees there is what other people are seeing. So I made some changes to make sure that when you remove your profile photo the UI updates right away, as well as fixed most flickering issues. --- .../jobs/RetrieveProfileAvatarJob.java | 13 ++-- .../securesms/profiles/AvatarHelper.java | 1 + .../manage/ManageProfileFragment.java | 8 ++- .../manage/ManageProfileRepository.java | 4 +- .../manage/ManageProfileViewModel.java | 59 ++++++++++++++----- .../securesms/util/ProfileUtil.java | 21 +++---- 6 files changed, 70 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java index acd2113462..e2ec5a7467 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java +++ b/app/src/main/java/org/thoughtcrime/securesms/jobs/RetrieveProfileAvatarJob.java @@ -81,19 +81,22 @@ public class RetrieveProfileAvatarJob extends BaseJob { return; } - if (Util.equals(profileAvatar, recipient.resolve().getProfileAvatar())) { + if (profileAvatar != null && profileAvatar.equals(recipient.resolve().getProfileAvatar())) { Log.w(TAG, "Already retrieved profile avatar: " + profileAvatar); return; } if (TextUtils.isEmpty(profileAvatar)) { - Log.w(TAG, "Removing profile avatar (no url) for: " + recipient.getId().serialize()); - AvatarHelper.delete(context, recipient.getId()); - database.setProfileAvatar(recipient.getId(), profileAvatar); + if (AvatarHelper.hasAvatar(context, recipient.getId())) { + Log.w(TAG, "Removing profile avatar (no url) for: " + recipient.getId().serialize()); + AvatarHelper.delete(context, recipient.getId()); + database.setProfileAvatar(recipient.getId(), profileAvatar); + } + return; } - File downloadDestination = File.createTempFile("avatar", "jpg", context.getCacheDir()); + File downloadDestination = File.createTempFile("avatar", "jpg", context.getCacheDir()); try { SignalServiceMessageReceiver receiver = ApplicationDependencies.getSignalServiceMessageReceiver(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/AvatarHelper.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/AvatarHelper.java index 93488ee59c..f2f0f6a7cb 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/AvatarHelper.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/AvatarHelper.java @@ -83,6 +83,7 @@ public class AvatarHelper { */ public static void delete(@NonNull Context context, @NonNull RecipientId recipientId) { getAvatarFile(context, recipientId).delete(); + Recipient.live(recipientId).refresh(); } /** diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileFragment.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileFragment.java index 5eaa53777b..cf9814e987 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileFragment.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileFragment.java @@ -39,10 +39,13 @@ import org.thoughtcrime.securesms.profiles.manage.ManageProfileViewModel.AvatarS import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.util.FeatureFlags; import org.thoughtcrime.securesms.util.NameUtil; +import org.thoughtcrime.securesms.util.livedata.LiveDataUtil; import org.thoughtcrime.securesms.util.navigation.SafeNavigation; import org.thoughtcrime.securesms.util.views.SimpleProgressDialog; import org.whispersystems.libsignal.util.guava.Optional; +import java.util.Arrays; + public class ManageProfileFragment extends LoggingFragment { private static final String TAG = Log.tag(ManageProfileFragment.class); @@ -137,7 +140,8 @@ public class ManageProfileFragment extends LoggingFragment { private void initializeViewModel() { viewModel = ViewModelProviders.of(this, new ManageProfileViewModel.Factory()).get(ManageProfileViewModel.class); - LiveData> avatarImage = Transformations.distinctUntilChanged(Transformations.map(viewModel.getAvatar(), avatar -> Optional.fromNullable(avatar.getAvatar()))); + LiveData> avatarImage = Transformations.map(LiveDataUtil.distinctUntilChanged(viewModel.getAvatar(), (b1, b2) -> Arrays.equals(b1.getAvatar(), b2.getAvatar())), + b -> Optional.fromNullable(b.getAvatar())); avatarImage.observe(getViewLifecycleOwner(), this::presentAvatarImage); viewModel.getAvatar().observe(getViewLifecycleOwner(), this::presentAvatarPlaceholder); @@ -161,7 +165,7 @@ public class ManageProfileFragment extends LoggingFragment { .circleCrop() .into(avatarView); } else { - avatarView.setImageDrawable(null); + Glide.with(this).load((Drawable) null).into(avatarView); } } diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileRepository.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileRepository.java index 5eb24168ca..a11987e9dc 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileRepository.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileRepository.java @@ -57,7 +57,7 @@ final class ManageProfileRepository { public void setAvatar(@NonNull Context context, @NonNull byte[] data, @NonNull String contentType, @NonNull Consumer callback) { SignalExecutors.UNBOUNDED.execute(() -> { try { - ProfileUtil.uploadProfileWithAvatar(context, new StreamDetails(new ByteArrayInputStream(data), contentType, data.length)); + ProfileUtil.uploadProfileWithAvatar(new StreamDetails(new ByteArrayInputStream(data), contentType, data.length)); AvatarHelper.setAvatar(context, Recipient.self().getId(), new ByteArrayInputStream(data)); SignalStore.misc().markHasEverHadAnAvatar(); ApplicationDependencies.getJobManager().add(new MultiDeviceProfileContentUpdateJob()); @@ -73,7 +73,7 @@ final class ManageProfileRepository { public void clearAvatar(@NonNull Context context, @NonNull Consumer callback) { SignalExecutors.UNBOUNDED.execute(() -> { try { - ProfileUtil.uploadProfileWithAvatar(context, null); + ProfileUtil.uploadProfileWithAvatar(null); AvatarHelper.delete(context, Recipient.self().getId()); ApplicationDependencies.getJobManager().add(new MultiDeviceProfileContentUpdateJob()); diff --git a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileViewModel.java b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileViewModel.java index c9e6f01e86..39a0f5765a 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileViewModel.java +++ b/app/src/main/java/org/thoughtcrime/securesms/profiles/manage/ManageProfileViewModel.java @@ -6,6 +6,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; +import androidx.lifecycle.Transformations; import androidx.lifecycle.ViewModel; import androidx.lifecycle.ViewModelProvider; @@ -30,6 +31,7 @@ import org.whispersystems.signalservice.api.util.StreamDetails; import java.io.IOException; import java.io.InputStream; +import java.util.Arrays; import java.util.Objects; class ManageProfileViewModel extends ViewModel { @@ -63,19 +65,6 @@ class ManageProfileViewModel extends ViewModel { SignalExecutors.BOUNDED.execute(() -> { onRecipientChanged(Recipient.self().fresh()); - - StreamDetails details = AvatarHelper.getSelfProfileAvatarStream(ApplicationDependencies.getApplication()); - if (details != null) { - try { - internalAvatarState.postValue(InternalAvatarState.loaded(StreamUtil.readFully(details.getStream()))); - } catch (IOException e) { - Log.w(TAG, "Failed to read avatar!"); - internalAvatarState.postValue(InternalAvatarState.none()); - } - } else { - internalAvatarState.postValue(InternalAvatarState.none()); - } - ApplicationDependencies.getJobManager().add(RetrieveProfileJob.forRecipient(Recipient.self().getId())); }); @@ -83,7 +72,7 @@ class ManageProfileViewModel extends ViewModel { } public @NonNull LiveData getAvatar() { - return avatarState; + return Transformations.distinctUntilChanged(avatarState); } public @NonNull LiveData getProfileName() { @@ -169,6 +158,20 @@ class ManageProfileViewModel extends ViewModel { about.postValue(recipient.getAbout()); aboutEmoji.postValue(recipient.getAboutEmoji()); badge.postValue(Optional.fromNullable(recipient.getFeaturedBadge())); + renderAvatar(AvatarHelper.getSelfProfileAvatarStream(ApplicationDependencies.getApplication())); + } + + private void renderAvatar(@Nullable StreamDetails details) { + if (details != null) { + try { + internalAvatarState.postValue(InternalAvatarState.loaded(StreamUtil.readFully(details.getStream()))); + } catch (IOException e) { + Log.w(TAG, "Failed to read avatar!"); + internalAvatarState.postValue(InternalAvatarState.none()); + } + } else { + internalAvatarState.postValue(InternalAvatarState.none()); + } } @Override @@ -198,6 +201,19 @@ class ManageProfileViewModel extends ViewModel { public @NonNull Recipient getSelf() { return self; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final AvatarState that = (AvatarState) o; + return Objects.equals(internalAvatarState, that.internalAvatarState) && Objects.equals(self, that.self); + } + + @Override + public int hashCode() { + return Objects.hash(internalAvatarState, self); + } } private final static class InternalAvatarState { @@ -228,6 +244,21 @@ class ManageProfileViewModel extends ViewModel { public LoadingState getLoadingState() { return loadingState; } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + final InternalAvatarState that = (InternalAvatarState) o; + return Arrays.equals(avatar, that.avatar) && loadingState == that.loadingState; + } + + @Override + public int hashCode() { + int result = Objects.hash(loadingState); + result = 31 * result + Arrays.hashCode(avatar); + return result; + } } public enum LoadingState { diff --git a/app/src/main/java/org/thoughtcrime/securesms/util/ProfileUtil.java b/app/src/main/java/org/thoughtcrime/securesms/util/ProfileUtil.java index f3d2400138..5471280ed9 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/util/ProfileUtil.java +++ b/app/src/main/java/org/thoughtcrime/securesms/util/ProfileUtil.java @@ -227,8 +227,7 @@ public final class ProfileUtil { public static void uploadProfileWithBadges(@NonNull Context context, @NonNull List badges) throws IOException { Log.d(TAG, "uploadProfileWithBadges()"); try (StreamDetails avatar = AvatarHelper.getSelfProfileAvatarStream(context)) { - uploadProfile(context, - Recipient.self().getProfileName(), + uploadProfile(Recipient.self().getProfileName(), Optional.fromNullable(Recipient.self().getAbout()).or(""), Optional.fromNullable(Recipient.self().getAboutEmoji()).or(""), getSelfPaymentsAddressProtobuf(), @@ -245,8 +244,7 @@ public final class ProfileUtil { public static void uploadProfileWithName(@NonNull Context context, @NonNull ProfileName profileName) throws IOException { Log.d(TAG, "uploadProfileWithName()"); try (StreamDetails avatar = AvatarHelper.getSelfProfileAvatarStream(context)) { - uploadProfile(context, - profileName, + uploadProfile(profileName, Optional.fromNullable(Recipient.self().getAbout()).or(""), Optional.fromNullable(Recipient.self().getAboutEmoji()).or(""), getSelfPaymentsAddressProtobuf(), @@ -263,8 +261,7 @@ public final class ProfileUtil { public static void uploadProfileWithAbout(@NonNull Context context, @NonNull String about, @NonNull String emoji) throws IOException { Log.d(TAG, "uploadProfileWithAbout()"); try (StreamDetails avatar = AvatarHelper.getSelfProfileAvatarStream(context)) { - uploadProfile(context, - Recipient.self().getProfileName(), + uploadProfile(Recipient.self().getProfileName(), about, emoji, getSelfPaymentsAddressProtobuf(), @@ -279,7 +276,7 @@ public final class ProfileUtil { public static void uploadProfile(@NonNull Context context) throws IOException { Log.d(TAG, "uploadProfile()"); try (StreamDetails avatar = AvatarHelper.getSelfProfileAvatarStream(context)) { - uploadProfileWithAvatar(context, avatar); + uploadProfileWithAvatar(avatar); } } @@ -288,10 +285,9 @@ public final class ProfileUtil { * avatar instead. This is useful when you want to ensure that the profile has been uploaded * successfully before persisting the change to disk. */ - public static void uploadProfileWithAvatar(@NonNull Context context, @Nullable StreamDetails avatar) throws IOException { + public static void uploadProfileWithAvatar(@Nullable StreamDetails avatar) throws IOException { Log.d(TAG, "uploadProfileWithAvatar()"); - uploadProfile(context, - Recipient.self().getProfileName(), + uploadProfile(Recipient.self().getProfileName(), Optional.fromNullable(Recipient.self().getAbout()).or(""), Optional.fromNullable(Recipient.self().getAboutEmoji()).or(""), getSelfPaymentsAddressProtobuf(), @@ -299,8 +295,7 @@ public final class ProfileUtil { Recipient.self().getBadges()); } - private static void uploadProfile(@NonNull Context context, - @NonNull ProfileName profileName, + private static void uploadProfile(@NonNull ProfileName profileName, @Nullable String about, @Nullable String aboutEmoji, @Nullable SignalServiceProtos.PaymentAddress paymentsAddress, @@ -308,7 +303,6 @@ public final class ProfileUtil { @NonNull List badges) throws IOException { - List badgeIds = badges.stream() .filter(Badge::getVisible) .map(Badge::getId) @@ -333,6 +327,7 @@ public final class ProfileUtil { badgeIds).orNull(); SignalStore.registrationValues().markHasUploadedProfile(); SignalDatabase.recipients().setProfileAvatar(Recipient.self().getId(), avatarPath); + ApplicationDependencies.getJobManager().add(new RefreshOwnProfileJob()); } private static @Nullable SignalServiceProtos.PaymentAddress getSelfPaymentsAddressProtobuf() {