Commit 9b7ca101 authored by Eleonora Rocchi's avatar Eleonora Rocchi Committed by Commit Bot

[PwdCheckAndroid] Add icon/progress bar to the header of Check Passwords.

This CL adds an icon/progress bar to the header of Check Passwords. It
also adds the tests to verify the correct visibility of the elements of
the header.

Screenshot in the linked bug.

Bug: 1109691,1092444
Change-Id: I329f9a21904288f1042417a67be9cb57583355f8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317875
Commit-Queue: Eleonora Rocchi <erocchi@google.com>
Reviewed-by: default avatarIoana Pandele <ioanap@chromium.org>
Reviewed-by: default avatarFriedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795868}
parent ea8523e3
......@@ -111,6 +111,7 @@ android_library("test_java") {
"//components/embedder_support/android:util_java",
"//content/public/test/android:content_java_test_support",
"//third_party/android_deps:androidx_annotation_annotation_java",
"//third_party/android_deps:androidx_appcompat_appcompat_resources_java",
"//third_party/android_deps:androidx_fragment_fragment_java",
"//third_party/android_deps:androidx_recyclerview_recyclerview_java",
"//third_party/android_deps:androidx_test_runner_java",
......
......@@ -93,9 +93,15 @@ generate_jni("jni_headers") {
}
android_resources("java_resources") {
deps = [ ":java_strings_grd" ]
deps = [
":java_strings_grd",
"//components/browser_ui/settings/android:java_resources",
"//components/browser_ui/styles/android:java_resources",
]
sources = [
"java/res/drawable/ic_autofill_assistant_white_24dp.xml",
"java/res/drawable/ic_check_circle_filled_green_24dp.xml",
"java/res/layout/password_check_compromised_credential_item.xml",
"java/res/layout/password_check_compromised_credential_with_script_item.xml",
"java/res/layout/password_check_header_item.xml",
......
<vector xmlns:android="http://schemas.android.com/apk/res/android"
android:width="24dp"
android:height="24dp"
android:viewportWidth="24"
android:viewportHeight="24">
<path
android:pathData="M12 2C6.48 2 2 6.48 2 12s4.48 10 10 10 10-4.48 10-10S17.52 2 12 2zm-2 15l-4-4 1.4-1.4 2.6 2.6 6.6-6.6L18 9l-8 8z"
android:fillColor="@color/default_green"/>
</vector>
......@@ -6,18 +6,67 @@
xmlns:app="http://schemas.android.com/apk/res-auto"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:gravity="end"
android:orientation="horizontal">
<ImageButton
android:id="@+id/check_status_restart_button"
android:background="?android:attr/selectableItemBackground"
android:contentDescription="@string/accessibility_password_check_restart_button"
android:layout_gravity="center_vertical"
android:layout_marginEnd="@dimen/check_status_restart_button_margin_end"
android:layout_height="@dimen/check_status_restart_button_clickable_surface_size"
android:layout_width="@dimen/check_status_restart_button_clickable_surface_size"
android:src="@drawable/ic_refresh_white_24dp"
app:tint="@color/default_icon_color"/>
android:orientation="vertical">
<LinearLayout
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:orientation="horizontal">
<ProgressBar
android:id="@+id/check_status_progress"
android:indeterminate="true"
android:layout_width="@dimen/check_status_icon_size"
android:layout_height="@dimen/check_status_icon_size"
android:layout_marginEnd="@dimen/check_status_icon_margin_horizontal"
android:layout_marginStart="@dimen/check_status_icon_margin_horizontal"
android:layout_gravity="center_vertical"
android:visibility="gone" />
<ImageView
android:importantForAccessibility="no"
android:id="@+id/check_status_icon"
android:layout_width="@dimen/check_status_icon_size"
android:layout_height="match_parent"
android:layout_marginEnd="@dimen/check_status_icon_margin_horizontal"
android:layout_marginStart="@dimen/check_status_icon_margin_horizontal"
android:visibility="gone" />
<LinearLayout
android:id="@+id/check_status_text_layout"
android:layout_height="wrap_content"
android:layout_width="0dp"
android:layout_weight="1"
android:orientation="vertical">
<TextView
android:id="@+id/check_status_message"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:textAppearance="@style/TextAppearance.TextLarge.Primary" />
<TextView
android:id="@+id/check_status_description"
android:layout_height="wrap_content"
android:layout_width="wrap_content"
android:textAppearance="@style/TextAppearance.TextSmall.Secondary"
android:visibility="gone" />
</LinearLayout>
<ImageButton
android:id="@+id/check_status_restart_button"
android:background="?android:attr/selectableItemBackground"
android:contentDescription="@string/accessibility_password_check_restart_button"
android:layout_gravity="center_vertical|end"
android:layout_marginEnd="@dimen/check_status_restart_button_margin_end"
android:layout_height="@dimen/check_status_restart_button_clickable_surface_size"
android:layout_width="@dimen/check_status_restart_button_clickable_surface_size"
android:src="@drawable/ic_refresh_white_24dp"
app:tint="@color/default_icon_color"/>
</LinearLayout>
<View style="@style/HorizontalDivider"/>
</LinearLayout>
\ No newline at end of file
......@@ -4,6 +4,8 @@
found in the LICENSE file. -->
<resources>
<dimen name="check_status_icon_margin_horizontal">16dp</dimen>
<dimen name="check_status_icon_size">24dp</dimen>
<dimen name="check_status_restart_button_clickable_surface_size">48dp</dimen>
<dimen name="check_status_restart_button_margin_end">4dp</dimen>
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.password_check;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import org.chromium.base.Consumer;
......@@ -34,6 +35,14 @@ class PasswordCheckMediator
void initialize(PropertyModel model, PasswordCheckComponentUi.Delegate delegate) {
mModel = model;
mDelegate = delegate;
ListModel<ListItem> items = mModel.get(ITEMS);
assert items.size() == 0;
items.add(new ListItem(PasswordCheckProperties.ItemType.HEADER,
new PropertyModel.Builder(PasswordCheckProperties.HeaderProperties.ALL_KEYS)
.with(CHECK_STATUS, PasswordCheckUIStatus.RUNNING)
.with(COMPROMISED_CREDENTIALS_COUNT, null)
.build()));
getPasswordCheck().addObserver(this, true);
}
......@@ -67,14 +76,13 @@ class PasswordCheckMediator
@Override
public void onPasswordCheckStatusChanged(@PasswordCheckUIStatus int status) {
ListModel<ListItem> items = mModel.get(ITEMS);
if (items.size() == 0) {
items.add(new ListItem(PasswordCheckProperties.ItemType.HEADER,
new PropertyModel.Builder(PasswordCheckProperties.HeaderProperties.ALL_KEYS)
.with(CHECK_STATUS, status)
.build()));
} else {
items.get(0).model.set(CHECK_STATUS, status);
}
assert items.size() >= 1;
PropertyModel header = items.get(0).model;
header.set(CHECK_STATUS, status);
// TODO(crbug.com/1109691): Retrieve the number of compromised credentials from the
// bridge.
header.set(COMPROMISED_CREDENTIALS_COUNT, 0);
}
@Override
......
......@@ -49,8 +49,11 @@ class PasswordCheckProperties {
static class HeaderProperties {
static final PropertyModel.WritableIntPropertyKey CHECK_STATUS =
new PropertyModel.WritableIntPropertyKey("check_status");
static final PropertyModel
.WritableObjectPropertyKey<Integer> COMPROMISED_CREDENTIALS_COUNT =
new PropertyModel.WritableObjectPropertyKey<>("compromised_credentials_count");
static final PropertyKey[] ALL_KEYS = {CHECK_STATUS};
static final PropertyKey[] ALL_KEYS = {CHECK_STATUS, COMPROMISED_CREDENTIALS_COUNT};
private HeaderProperties() {}
}
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.password_check;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import static org.chromium.components.embedder_support.util.UrlUtilities.stripScheme;
......@@ -14,6 +15,7 @@ import android.content.Context;
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageButton;
import android.widget.ImageView;
import android.widget.TextView;
import org.chromium.chrome.browser.password_check.PasswordCheckProperties.ItemType;
......@@ -152,22 +154,15 @@ class PasswordCheckViewBinder {
* @param key The {@link PropertyKey} which changed.
*/
private static void bindHeaderView(PropertyModel model, View view, PropertyKey key) {
@PasswordCheckUIStatus
int status = model.get(CHECK_STATUS);
Integer compromisedCredentialsCount = model.get(COMPROMISED_CREDENTIALS_COUNT);
if (key == CHECK_STATUS) {
// TODO(crbug.com/1101256): Set text and illustration based on status.
@PasswordCheckUIStatus
int status = model.get(CHECK_STATUS);
ImageButton restartButton = view.findViewById(R.id.check_status_restart_button);
if (status != PasswordCheckUIStatus.RUNNING) {
restartButton.setVisibility(View.VISIBLE);
restartButton.setClickable(true);
restartButton.setOnClickListener(unusedView
-> {
// TODO(crbug.com/1092444): Add call to restart the check.
});
} else {
restartButton.setVisibility(View.GONE);
restartButton.setClickable(false);
}
// TODO(crbug.com/1109691): Set text and illustration based on status.
updateActionButton(view, status);
updateStatusIcon(view, status, compromisedCredentialsCount);
} else if (key == COMPROMISED_CREDENTIALS_COUNT) {
updateStatusIcon(view, status, compromisedCredentialsCount);
} else {
assert false : "Unhandled update to property:" + key;
}
......@@ -175,6 +170,62 @@ class PasswordCheckViewBinder {
private PasswordCheckViewBinder() {}
private static void updateActionButton(View view, @PasswordCheckUIStatus int status) {
ImageButton restartButton = view.findViewById(R.id.check_status_restart_button);
if (status != PasswordCheckUIStatus.RUNNING) {
restartButton.setVisibility(View.VISIBLE);
restartButton.setClickable(true);
restartButton.setOnClickListener(unusedView
-> {
// TODO(crbug.com/1092444): Add call to restart the check.
});
} else {
restartButton.setVisibility(View.GONE);
restartButton.setClickable(false);
}
}
private static void updateStatusIcon(
View view, @PasswordCheckUIStatus int status, Integer compromisedCredentialsCount) {
if (status == PasswordCheckUIStatus.IDLE && compromisedCredentialsCount == null) return;
ImageView statusIcon = view.findViewById(R.id.check_status_icon);
statusIcon.setImageResource(getIconResource(status, compromisedCredentialsCount));
statusIcon.setVisibility(getIconVisibility(status));
view.findViewById(R.id.check_status_progress)
.setVisibility(getProgressBarVisibility(status));
}
private static int getIconResource(
@PasswordCheckUIStatus int status, Integer compromisedCredentialsCount) {
switch (status) {
case PasswordCheckUIStatus.IDLE:
assert compromisedCredentialsCount != null;
return compromisedCredentialsCount == 0
? R.drawable.ic_check_circle_filled_green_24dp
: org.chromium.chrome.R.drawable.ic_warning_red_24dp;
case PasswordCheckUIStatus.RUNNING:
return 0;
case PasswordCheckUIStatus.ERROR_OFFLINE:
case PasswordCheckUIStatus.ERROR_NO_PASSWORDS:
case PasswordCheckUIStatus.ERROR_SIGNED_OUT:
case PasswordCheckUIStatus.ERROR_QUOTA_LIMIT:
case PasswordCheckUIStatus.ERROR_QUOTA_LIMIT_ACCOUNT_CHECK:
case PasswordCheckUIStatus.ERROR_UNKNOWN:
return org.chromium.chrome.R.drawable.ic_error_grey800_24dp_filled;
default:
assert false : "Unhandled check status " + status + "on icon update";
}
return 0;
}
private static int getIconVisibility(@PasswordCheckUIStatus int status) {
return status == PasswordCheckUIStatus.RUNNING ? View.GONE : View.VISIBLE;
}
private static int getProgressBarVisibility(@PasswordCheckUIStatus int status) {
return status == PasswordCheckUIStatus.RUNNING ? View.VISIBLE : View.GONE;
}
private static ListMenu createCredentialMenu(Context context, CompromisedCredential credential,
PasswordCheckCoordinator.CredentialEventHandler credentialHandler) {
MVCListAdapter.ModelList menuItems = new MVCListAdapter.ModelList();
......
......@@ -70,6 +70,7 @@ public class PasswordCheckIntegrationTest {
@Test
@MediumTest
@DisabledTest(message = "crbug.com/1114096")
public void testDoesNotDestroyComponentIfNotFirstInSettingsStack() {
PasswordCheckFactory.getOrCreate();
SettingsActivity activity = setUpUiLaunchedFromSettings();
......
......@@ -11,8 +11,10 @@ import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.timeout;
import static org.mockito.Mockito.verify;
......@@ -20,16 +22,26 @@ import static org.mockito.Mockito.verify;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.COMPROMISED_CREDENTIAL;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.CompromisedCredentialProperties.CREDENTIAL_HANDLER;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.CHECK_STATUS;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.HeaderProperties.COMPROMISED_CREDENTIALS_COUNT;
import static org.chromium.chrome.browser.password_check.PasswordCheckProperties.ITEMS;
import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.ERROR_OFFLINE;
import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.IDLE;
import static org.chromium.chrome.browser.password_check.PasswordCheckUIStatus.RUNNING;
import static org.chromium.content_public.browser.test.util.CriteriaHelper.pollUiThread;
import static org.chromium.content_public.browser.test.util.TestThreadUtils.runOnUiThreadBlocking;
import android.graphics.Bitmap;
import android.graphics.Canvas;
import android.graphics.drawable.Drawable;
import android.os.Bundle;
import android.view.View;
import android.widget.ImageButton;
import android.widget.ImageView;
import android.widget.ProgressBar;
import android.widget.TextView;
import androidx.annotation.IdRes;
import androidx.appcompat.content.res.AppCompatResources;
import androidx.recyclerview.widget.RecyclerView;
import androidx.test.filters.MediumTest;
......@@ -100,10 +112,10 @@ public class PasswordCheckViewTest {
@MediumTest
public void testDisplaysHeaderAndCredential() {
runOnUiThreadBlocking(() -> {
mModel.get(ITEMS).add(buildHeader(PasswordCheckUIStatus.IDLE));
mModel.get(ITEMS).add(buildHeader(RUNNING));
mModel.get(ITEMS).add(buildCredentialItem(ANA));
});
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(2)));
waitForListViewToHaveLength(2);
// Has a change passwords button.
assertNotNull(getCredentialChangeButtonAt(1));
assertThat(getCredentialChangeButtonAt(1).getVisibility(), is(View.VISIBLE));
......@@ -117,22 +129,55 @@ public class PasswordCheckViewTest {
is(getString(org.chromium.chrome.R.string.more)));
}
@Test
@MediumTest
public void testStatusDisplaysIconOnIdleNoLeaks() {
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(IDLE, 0)); });
waitForListViewToHaveLength(1);
assertDisplaysIcon(R.drawable.ic_check_circle_filled_green_24dp);
}
@Test
@MediumTest
public void testStatusDisplaysIconOnIdleWithLeaks() {
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(IDLE, 2)); });
waitForListViewToHaveLength(1);
assertDisplaysIcon(org.chromium.chrome.R.drawable.ic_warning_red_24dp);
}
@Test
@MediumTest
public void testStatusDisplaysIconOnError() {
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(ERROR_OFFLINE)); });
waitForListViewToHaveLength(1);
assertDisplaysIcon(org.chromium.chrome.R.drawable.ic_error_grey800_24dp_filled);
}
@Test
@MediumTest
public void testStatusDisplaysProgressBarOnRunning() {
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(RUNNING)); });
waitForListViewToHaveLength(1);
assertThat(getHeaderIcon().getVisibility(), is(View.GONE));
assertThat(getHeaderProgressBar().getVisibility(), is(View.VISIBLE));
}
@Test
@MediumTest
public void testStatusDisplaysRestartAction() {
runOnUiThreadBlocking(
() -> { mModel.get(ITEMS).add(buildHeader(PasswordCheckUIStatus.IDLE)); });
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(1)));
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(IDLE, 0)); });
waitForListViewToHaveLength(1);
assertThat(getActionButton().getVisibility(), is(View.VISIBLE));
assertTrue(getActionButton().isClickable());
}
@Test
@MediumTest
public void testStatusNotDisplaysRestartAction() {
runOnUiThreadBlocking(
() -> { mModel.get(ITEMS).add(buildHeader(PasswordCheckUIStatus.RUNNING)); });
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(1)));
runOnUiThreadBlocking(() -> { mModel.get(ITEMS).add(buildHeader(RUNNING)); });
waitForListViewToHaveLength(1);
assertThat(getActionButton().getVisibility(), is(View.GONE));
assertFalse(getActionButton().isClickable());
}
@Test
......@@ -142,7 +187,7 @@ public class PasswordCheckViewTest {
mModel.get(ITEMS).add(buildCredentialItem(PHISHED));
mModel.get(ITEMS).add(buildCredentialItem(LEAKED));
});
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(2)));
waitForListViewToHaveLength(2);
// The phished credential is rendered first:
assertThat(getCredentialOriginAt(0).getText(), is(PHISHED.getOriginUrl()));
......@@ -214,7 +259,7 @@ public class PasswordCheckViewTest {
@MediumTest
public void testClickingDeleteInMoreMenuTriggersHandler() {
runOnUiThreadBlocking(() -> mModel.get(ITEMS).add(buildCredentialItem(ANA)));
pollUiThread(() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(1)));
waitForListViewToHaveLength(1);
TouchCommon.singleClickView(getCredentialMoreButtonAt(0));
......@@ -227,9 +272,15 @@ public class PasswordCheckViewTest {
}
private MVCListAdapter.ListItem buildHeader(@PasswordCheckUIStatus int status) {
return buildHeader(status, null);
}
private MVCListAdapter.ListItem buildHeader(
@PasswordCheckUIStatus int status, Integer compromisedCredentialsCount) {
return new MVCListAdapter.ListItem(PasswordCheckProperties.ItemType.HEADER,
new PropertyModel.Builder(HeaderProperties.ALL_KEYS)
.with(CHECK_STATUS, status)
.with(COMPROMISED_CREDENTIALS_COUNT, compromisedCredentialsCount)
.build());
}
......@@ -251,10 +302,35 @@ public class PasswordCheckViewTest {
mTestRule.startSettingsActivity(fragmentArgs);
}
private void waitForListViewToHaveLength(int length) {
pollUiThread(
() -> Criteria.checkThat(getPasswordCheckViewList().getChildCount(), is(length)));
}
private void assertDisplaysIcon(int resourceId) {
assertThat(getHeaderIcon().getVisibility(), is(View.VISIBLE));
assertThat(getHeaderProgressBar().getVisibility(), is(View.GONE));
Drawable icon = getHeaderIcon().getDrawable();
int widthPx = icon.getIntrinsicWidth();
int heightPx = icon.getIntrinsicHeight();
assertTrue(getBitmap(
AppCompatResources.getDrawable(mPasswordCheckView.getContext(), resourceId),
widthPx, heightPx)
.sameAs(getBitmap(icon, widthPx, heightPx)));
}
private View getStatus() {
return mPasswordCheckView.getListView().getChildAt(0);
}
private ImageView getHeaderIcon() {
return getStatus().findViewById(R.id.check_status_icon);
}
private ProgressBar getHeaderProgressBar() {
return getStatus().findViewById(R.id.check_status_progress);
}
private ImageButton getActionButton() {
return getStatus().findViewById(R.id.check_status_restart_button);
}
......@@ -303,4 +379,12 @@ public class PasswordCheckViewTest {
return verify(mock,
timeout(ScalableTimeout.scaleTimeout(CriteriaHelper.DEFAULT_MAX_TIME_TO_POLL)));
}
private Bitmap getBitmap(Drawable drawable, int widthPx, int heightPx) {
Bitmap bitmap = Bitmap.createBitmap(widthPx, heightPx, Bitmap.Config.ARGB_8888);
Canvas canvas = new Canvas(bitmap);
drawable.setBounds(0, 0, widthPx, heightPx);
drawable.draw(canvas);
return bitmap;
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment