Fix account deletion UI bugs.

This commit is contained in:
Alex Hart 2020-12-08 10:42:16 -04:00 committed by Greyson Parrelli
parent bfa56f771d
commit f1d0d4f81b
7 changed files with 176 additions and 173 deletions

View file

@ -8,11 +8,13 @@ final class Country {
private final String displayName; private final String displayName;
private final int code; private final int code;
private final String normalized; private final String normalized;
private final String region;
Country(@NonNull String displayName, int code) { Country(@NonNull String displayName, int code, @NonNull String region) {
this.displayName = displayName; this.displayName = displayName;
this.code = code; this.code = code;
this.normalized = displayName.toLowerCase(); this.normalized = displayName.toLowerCase();
this.region = region;
} }
int getCode() { int getCode() {
@ -27,6 +29,10 @@ final class Country {
return normalized; return normalized;
} }
@NonNull String getRegion() {
return region;
}
@Override @Override
public boolean equals(Object o) { public boolean equals(Object o) {
if (this == o) return true; if (this == o) return true;

View file

@ -16,7 +16,6 @@ import androidx.lifecycle.ViewModelProviders;
import androidx.recyclerview.widget.RecyclerView; import androidx.recyclerview.widget.RecyclerView;
import org.thoughtcrime.securesms.R; import org.thoughtcrime.securesms.R;
import org.thoughtcrime.securesms.util.ThemeUtil;
import org.thoughtcrime.securesms.util.text.AfterTextChanged; import org.thoughtcrime.securesms.util.text.AfterTextChanged;
public class DeleteAccountCountryPickerFragment extends DialogFragment { public class DeleteAccountCountryPickerFragment extends DialogFragment {
@ -61,7 +60,7 @@ public class DeleteAccountCountryPickerFragment extends DialogFragment {
} }
private void onCountryPicked(@NonNull Country country) { private void onCountryPicked(@NonNull Country country) {
viewModel.onCountrySelected(country.getDisplayName(), country.getCode()); viewModel.onRegionSelected(country.getRegion());
dismissAllowingStateLoss(); dismissAllowingStateLoss();
} }
} }

View file

@ -4,6 +4,7 @@ import android.annotation.SuppressLint;
import android.app.AlertDialog; import android.app.AlertDialog;
import android.content.DialogInterface; import android.content.DialogInterface;
import android.content.Intent; import android.content.Intent;
import android.graphics.Color;
import android.net.Uri; import android.net.Uri;
import android.os.Bundle; import android.os.Bundle;
import android.provider.Settings; import android.provider.Settings;
@ -64,8 +65,7 @@ public class DeleteAccountFragment extends Fragment {
viewModel = ViewModelProviders.of(requireActivity(), new DeleteAccountViewModel.Factory(new DeleteAccountRepository())) viewModel = ViewModelProviders.of(requireActivity(), new DeleteAccountViewModel.Factory(new DeleteAccountRepository()))
.get(DeleteAccountViewModel.class); .get(DeleteAccountViewModel.class);
viewModel.getCountryDisplayName().observe(getViewLifecycleOwner(), this::setCountryDisplay); viewModel.getCountryDisplayName().observe(getViewLifecycleOwner(), this::setCountryDisplay);
viewModel.getRegionCode().observe(getViewLifecycleOwner(), this::setCountryFormatter); viewModel.getRegionCode().observe(getViewLifecycleOwner(), this::handleRegionUpdated);
viewModel.getCountryCode().observe(getViewLifecycleOwner(), this::setCountryCode);
viewModel.getEvents().observe(getViewLifecycleOwner(), this::handleEvent); viewModel.getEvents().observe(getViewLifecycleOwner(), this::handleEvent);
initializeNumberInput(); initializeNumberInput();
@ -87,9 +87,7 @@ public class DeleteAccountFragment extends Fragment {
private @NonNull CharSequence buildBulletsText() { private @NonNull CharSequence buildBulletsText() {
return new SpannableStringBuilder().append(SpanUtil.bullet(getString(R.string.DeleteAccountFragment__delete_your_account_info_and_profile_photo))) return new SpannableStringBuilder().append(SpanUtil.bullet(getString(R.string.DeleteAccountFragment__delete_your_account_info_and_profile_photo)))
.append("\n") .append("\n")
.append(SpanUtil.bullet(getString(R.string.DeleteAccountFragment__delete_all_your_messages))) .append(SpanUtil.bullet(getString(R.string.DeleteAccountFragment__delete_all_your_messages)));
.append("\n")
.append(SpanUtil.bullet(getString(R.string.DeleteAccountFragment__remove_you_from_all_signal_groups)));
} }
@SuppressLint("ClickableViewAccessibility") @SuppressLint("ClickableViewAccessibility")
@ -114,13 +112,10 @@ public class DeleteAccountFragment extends Fragment {
} }
private void pickCountry() { private void pickCountry() {
countryCode.clearFocus();
DeleteAccountCountryPickerFragment.show(requireFragmentManager()); DeleteAccountCountryPickerFragment.show(requireFragmentManager());
} }
private void setCountryCode(int countryCode) {
this.countryCode.setText(String.valueOf(countryCode));
}
private void setCountryDisplay(@NonNull String regionDisplayName) { private void setCountryDisplay(@NonNull String regionDisplayName) {
countrySpinnerAdapter.clear(); countrySpinnerAdapter.clear();
if (TextUtils.isEmpty(regionDisplayName)) { if (TextUtils.isEmpty(regionDisplayName)) {
@ -130,18 +125,20 @@ public class DeleteAccountFragment extends Fragment {
} }
} }
private void setCountryFormatter(@Nullable String regionCode) { private void handleRegionUpdated(@Nullable String regionCode) {
PhoneNumberUtil util = PhoneNumberUtil.getInstance(); PhoneNumberUtil util = PhoneNumberUtil.getInstance();
countryFormatter = regionCode != null ? util.getAsYouTypeFormatter(regionCode) : null; countryFormatter = regionCode != null ? util.getAsYouTypeFormatter(regionCode) : null;
reformatText(number.getText()); reformatText(number.getText());
if (!TextUtils.isEmpty(regionCode) && !regionCode.equals("ZZ")) { if (!TextUtils.isEmpty(regionCode) && !"ZZ".equals(regionCode) && !countryCode.hasFocus()) {
number.requestFocus(); number.requestFocus();
int numberLength = number.getText().length(); int numberLength = number.getText().length();
number.getInput().setSelection(numberLength, numberLength); number.getInput().setSelection(numberLength, numberLength);
countryCode.setText(String.valueOf(util.getCountryCodeForRegion(regionCode)));
} }
} }
@ -202,11 +199,11 @@ public class DeleteAccountFragment extends Fragment {
private void afterCountryCodeChanged(@Nullable Editable s) { private void afterCountryCodeChanged(@Nullable Editable s) {
if (TextUtils.isEmpty(s) || !TextUtils.isDigitsOnly(s)) { if (TextUtils.isEmpty(s) || !TextUtils.isDigitsOnly(s)) {
viewModel.onCountrySelected(null, 0); viewModel.onCountrySelected(0);
return; return;
} }
viewModel.onCountrySelected(null, Integer.parseInt(s.toString())); viewModel.onCountrySelected(Integer.parseInt(s.toString()));
} }
private void afterNumberChanged(@Nullable Editable s) { private void afterNumberChanged(@Nullable Editable s) {
@ -220,10 +217,10 @@ public class DeleteAccountFragment extends Fragment {
private void handleEvent(@NonNull DeleteAccountViewModel.EventType eventType) { private void handleEvent(@NonNull DeleteAccountViewModel.EventType eventType) {
switch (eventType) { switch (eventType) {
case NO_COUNTRY_CODE: case NO_COUNTRY_CODE:
Snackbar.make(requireView(), R.string.DeleteAccountFragment__no_country_code, Snackbar.LENGTH_SHORT).show(); Snackbar.make(requireView(), R.string.DeleteAccountFragment__no_country_code, Snackbar.LENGTH_SHORT).setTextColor(Color.WHITE).show();
break; break;
case NO_NATIONAL_NUMBER: case NO_NATIONAL_NUMBER:
Snackbar.make(requireView(), R.string.DeleteAccountFragment__no_number, Snackbar.LENGTH_SHORT).show(); Snackbar.make(requireView(), R.string.DeleteAccountFragment__no_number, Snackbar.LENGTH_SHORT).setTextColor(Color.WHITE).show();
break; break;
case NOT_A_MATCH: case NOT_A_MATCH:
new AlertDialog.Builder(requireContext()) new AlertDialog.Builder(requireContext())

View file

@ -28,6 +28,14 @@ class DeleteAccountRepository {
.toList(); .toList();
} }
@NonNull String getRegionDisplayName(@NonNull String region) {
return PhoneNumberFormatter.getRegionDisplayName(region);
}
int getRegionCountryCode(@NonNull String region) {
return PhoneNumberUtil.getInstance().getCountryCodeForRegion(region);
}
void deleteAccount(@NonNull Runnable onFailureToRemovePin, void deleteAccount(@NonNull Runnable onFailureToRemovePin,
@NonNull Runnable onFailureToDeleteFromService, @NonNull Runnable onFailureToDeleteFromService,
@NonNull Runnable onFailureToDeleteLocalData) @NonNull Runnable onFailureToDeleteLocalData)
@ -66,7 +74,8 @@ class DeleteAccountRepository {
private static @NonNull Country getCountryForRegion(@NonNull String region) { private static @NonNull Country getCountryForRegion(@NonNull String region) {
return new Country(PhoneNumberFormatter.getRegionDisplayName(region), return new Country(PhoneNumberFormatter.getRegionDisplayName(region),
PhoneNumberUtil.getInstance().getCountryCodeForRegion(region)); PhoneNumberUtil.getInstance().getCountryCodeForRegion(region),
region);
} }
private static class RegionComparator implements Comparator<Country> { private static class RegionComparator implements Comparator<Country> {

View file

@ -11,14 +11,15 @@ import androidx.lifecycle.ViewModel;
import androidx.lifecycle.ViewModelProvider; import androidx.lifecycle.ViewModelProvider;
import com.annimon.stream.Stream; import com.annimon.stream.Stream;
import com.google.i18n.phonenumbers.NumberParseException;
import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.i18n.phonenumbers.PhoneNumberUtil;
import com.google.i18n.phonenumbers.Phonenumber; import com.google.i18n.phonenumbers.Phonenumber;
import org.signal.core.util.logging.Log;
import org.thoughtcrime.securesms.recipients.Recipient; import org.thoughtcrime.securesms.recipients.Recipient;
import org.thoughtcrime.securesms.registration.viewmodel.NumberViewState;
import org.thoughtcrime.securesms.util.DefaultValueLiveData; import org.thoughtcrime.securesms.util.DefaultValueLiveData;
import org.thoughtcrime.securesms.util.SingleLiveEvent; import org.thoughtcrime.securesms.util.SingleLiveEvent;
import org.thoughtcrime.securesms.util.livedata.LiveDataUtil; import org.whispersystems.signalservice.api.util.PhoneNumberFormatter;
import java.util.List; import java.util.List;
@ -27,29 +28,20 @@ public class DeleteAccountViewModel extends ViewModel {
private final DeleteAccountRepository repository; private final DeleteAccountRepository repository;
private final List<Country> allCountries; private final List<Country> allCountries;
private final LiveData<List<Country>> filteredCountries; private final LiveData<List<Country>> filteredCountries;
private final LiveData<String> regionCode; private final MutableLiveData<String> regionCode;
private final MutableLiveData<Integer> countryCode; private final LiveData<String> countryDisplayName;
private final MutableLiveData<String> countryDisplayName;
private final MutableLiveData<Long> nationalNumber; private final MutableLiveData<Long> nationalNumber;
private final MutableLiveData<String> query; private final MutableLiveData<String> query;
private final SingleLiveEvent<EventType> events; private final SingleLiveEvent<EventType> events;
private final LiveData<NumberViewState> numberViewState;
public DeleteAccountViewModel(@NonNull DeleteAccountRepository repository) { public DeleteAccountViewModel(@NonNull DeleteAccountRepository repository) {
this.repository = repository; this.repository = repository;
this.allCountries = repository.getAllCountries(); this.allCountries = repository.getAllCountries();
this.countryCode = new DefaultValueLiveData<>(NumberViewState.INITIAL.getCountryCode()); this.regionCode = new DefaultValueLiveData<>(PhoneNumberUtil.REGION_CODE_FOR_NON_GEO_ENTITY);
this.nationalNumber = new DefaultValueLiveData<>(NumberViewState.INITIAL.getNationalNumber()); this.nationalNumber = new MutableLiveData<>();
this.countryDisplayName = new DefaultValueLiveData<>(NumberViewState.INITIAL.getCountryDisplayName());
this.query = new DefaultValueLiveData<>(""); this.query = new DefaultValueLiveData<>("");
this.regionCode = Transformations.map(countryCode, this::mapCountryCodeToRegionCode); this.countryDisplayName = Transformations.map(regionCode, repository::getRegionDisplayName);
this.filteredCountries = Transformations.map(query, q -> Stream.of(allCountries).filter(country -> isMatch(q, country)).toList()); this.filteredCountries = Transformations.map(query, q -> Stream.of(allCountries).filter(country -> isMatch(q, country)).toList());
LiveData<NumberViewState> partialViewState = LiveDataUtil.combineLatest(countryCode,
countryDisplayName,
DeleteAccountViewModel::getPartialNumberViewState);
this.numberViewState = LiveDataUtil.combineLatest(partialViewState, nationalNumber, DeleteAccountViewModel::getCompleteNumberViewState);
this.events = new SingleLiveEvent<>(); this.events = new SingleLiveEvent<>();
} }
@ -58,28 +50,19 @@ public class DeleteAccountViewModel extends ViewModel {
} }
@NonNull LiveData<String> getCountryDisplayName() { @NonNull LiveData<String> getCountryDisplayName() {
return Transformations.distinctUntilChanged(Transformations.map(numberViewState, NumberViewState::getCountryDisplayName)); return Transformations.distinctUntilChanged(countryDisplayName);
} }
@NonNull LiveData<String> getRegionCode() { @NonNull LiveData<String> getRegionCode() {
return Transformations.distinctUntilChanged(regionCode); return Transformations.distinctUntilChanged(regionCode);
} }
@NonNull LiveData<Integer> getCountryCode() {
return Transformations.distinctUntilChanged(Transformations.map(numberViewState, NumberViewState::getCountryCode));
}
@NonNull SingleLiveEvent<EventType> getEvents() { @NonNull SingleLiveEvent<EventType> getEvents() {
return events; return events;
} }
@Nullable Long getNationalNumber() { @Nullable Long getNationalNumber() {
Long number = nationalNumber.getValue(); return nationalNumber.getValue();
if (number == null || number == NumberViewState.INITIAL.getNationalNumber()) {
return null;
} else {
return number;
}
} }
void onQueryChanged(@NonNull String query) { void onQueryChanged(@NonNull String query) {
@ -93,7 +76,8 @@ public class DeleteAccountViewModel extends ViewModel {
} }
void submit() { void submit() {
Integer countryCode = this.countryCode.getValue(); String region = this.regionCode.getValue();
Integer countryCode = region != null ? repository.getRegionCountryCode(region) : null;
Long nationalNumber = this.nationalNumber.getValue(); Long nationalNumber = this.nationalNumber.getValue();
if (countryCode == null || countryCode == 0) { if (countryCode == null || countryCode == 0) {
@ -117,28 +101,31 @@ public class DeleteAccountViewModel extends ViewModel {
} }
} }
void onCountrySelected(@Nullable String countryDisplayName, int countryCode) { void onCountrySelected(int countryCode) {
if (countryDisplayName != null) { String region = this.regionCode.getValue();
this.countryDisplayName.setValue(countryDisplayName); List<String> regions = PhoneNumberUtil.getInstance().getRegionCodesForCountryCode(countryCode);
if (!regions.contains(region)) {
this.regionCode.setValue(PhoneNumberUtil.getInstance().getRegionCodeForCountryCode(countryCode));
}
} }
this.countryCode.setValue(countryCode); void onRegionSelected(@NonNull String region) {
this.regionCode.setValue(region);
} }
void setNationalNumber(long nationalNumber) { void setNationalNumber(long nationalNumber) {
this.nationalNumber.setValue(nationalNumber); this.nationalNumber.setValue(nationalNumber);
}
private @NonNull String mapCountryCodeToRegionCode(int countryCode) { try {
return PhoneNumberUtil.getInstance().getRegionCodeForCountryCode(countryCode); String phoneNumberRegion = PhoneNumberUtil.getInstance()
.getRegionCodeForNumber(PhoneNumberUtil.getInstance().parse(String.valueOf(nationalNumber),
regionCode.getValue()));
if (phoneNumberRegion != null) {
regionCode.setValue(phoneNumberRegion);
} }
} catch (NumberParseException ignored) {
private static @NonNull NumberViewState getPartialNumberViewState(int countryCode, @Nullable String countryDisplayName) {
return new NumberViewState.Builder().countryCode(countryCode).selectedCountryDisplayName(countryDisplayName).build();
} }
private static @NonNull NumberViewState getCompleteNumberViewState(@NonNull NumberViewState partial, long nationalNumber) {
return partial.toBuilder().nationalNumber(nationalNumber).build();
} }
private static boolean isMatch(@NonNull String query, @NonNull Country country) { private static boolean isMatch(@NonNull String query, @NonNull Country country) {

View file

@ -1,9 +1,14 @@
<?xml version="1.0" encoding="utf-8"?> <?xml version="1.0" encoding="utf-8"?>
<androidx.constraintlayout.widget.ConstraintLayout xmlns:android="http://schemas.android.com/apk/res/android" <ScrollView xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto" xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools" xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="match_parent"> android:layout_height="match_parent"
android:fillViewport="true">
<androidx.constraintlayout.widget.ConstraintLayout
android:layout_width="match_parent"
android:layout_height="wrap_content">
<androidx.appcompat.widget.AppCompatImageView <androidx.appcompat.widget.AppCompatImageView
android:id="@+id/delete_account_fragment_warning" android:id="@+id/delete_account_fragment_warning"
@ -108,8 +113,8 @@
</LinearLayout> </LinearLayout>
<com.google.android.material.button.MaterialButton <com.google.android.material.button.MaterialButton
style="@style/Signal.Widget.Button.Large.Danger"
android:id="@+id/delete_account_fragment_delete" android:id="@+id/delete_account_fragment_delete"
style="@style/Signal.Widget.Button.Large.Danger"
android:layout_width="match_parent" android:layout_width="match_parent"
android:layout_height="wrap_content" android:layout_height="wrap_content"
android:layout_marginStart="32dp" android:layout_marginStart="32dp"
@ -121,3 +126,4 @@
app:layout_constraintTop_toBottomOf="@id/delete_account_fragment_linearLayout" /> app:layout_constraintTop_toBottomOf="@id/delete_account_fragment_linearLayout" />
</androidx.constraintlayout.widget.ConstraintLayout> </androidx.constraintlayout.widget.ConstraintLayout>
</ScrollView>

View file

@ -2994,7 +2994,6 @@
<string name="DeleteAccountFragment__delete_account">Delete account</string> <string name="DeleteAccountFragment__delete_account">Delete account</string>
<string name="DeleteAccountFragment__delete_your_account_info_and_profile_photo">Delete your account info and profile photo</string> <string name="DeleteAccountFragment__delete_your_account_info_and_profile_photo">Delete your account info and profile photo</string>
<string name="DeleteAccountFragment__delete_all_your_messages">Delete all your messages</string> <string name="DeleteAccountFragment__delete_all_your_messages">Delete all your messages</string>
<string name="DeleteAccountFragment__remove_you_from_all_signal_groups">Remove you from all Signal groups</string>
<string name="DeleteAccountFragment__no_country_code">No country code specified</string> <string name="DeleteAccountFragment__no_country_code">No country code specified</string>
<string name="DeleteAccountFragment__no_number">No number specified</string> <string name="DeleteAccountFragment__no_number">No number specified</string>
<string name="DeleteAccountFragment__the_phone_number">The phone number you entered doesn\'t match your account\'s.</string> <string name="DeleteAccountFragment__the_phone_number">The phone number you entered doesn\'t match your account\'s.</string>