Commit 580edb0e authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

[Android][TouchToFill] Add confirmation button for single list entries

This CL adds a blue "Use Password" button to the touch to fill sheet if
exactly one suggestion exists. This should address the potential problem
that users don't see the bottom sheet as actionable.

This CL can only be merged if a string translation is approved.

The button is guarded by a field trial param. Activate it with a study
like this:

    "TouchToFillStudy": [
        {
          "platforms": ["android"],
          "experiments": [
              {
                  "name": "Enabled",
                  "params": {
                      "show_confirmation_button": "true"
                  },
                  "enable_features": [
                      "TouchToFillAndroid"
                  ]
              }
          ]
        }
    ]

Screenshots with and without button are attached to the initial
comment of the linked bug.

Bug: 1021506
Change-Id: I8857987d7726800846d58338a518317099def022
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1892962Reviewed-by: default avatarJan Wilken Dörrie <jdoerrie@chromium.org>
Commit-Queue: Friedrich [CET] <fhorschig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713403}
parent e6c048fd
<?xml version="1.0" encoding="utf-8"?>
<!-- Copyright 2019 The Chromium Authors. All rights reserved.
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file. -->
<org.chromium.ui.widget.ButtonCompat
xmlns:android="http://schemas.android.com/apk/res/android"
android:descendantFocusability="blocksDescendants"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:layout_marginBottom="16dp"
android:minHeight="48dp"
android:gravity="center"
android:text="@string/touch_to_fill_confirm"
android:background="@drawable/touch_to_fill_credential_background"
android:ellipsize="end"
android:singleLine="true"
style="@style/FilledButton.Flat"/>
......@@ -6,6 +6,21 @@
<resources>
<dimen name="touch_to_fill_favicon_size">24dp</dimen>
<dimen name="touch_to_fill_sheet_margin">16dp</dimen>
<!-- Below are the different Half-state peeking heights. The height is the
sum of all components. It varies depending on the suggestion count. The
base height is 282dp:
Bottom sheet shadows and padding (18dp)
+ Handlebar (16dp)
+ Header size (112 dp)
+ Title size and margin (48+16dp)
+ First suggestion (72dp) -->
<!-- Base height (282dp) plus padding (8dp) plus half suggestion (36dp) -->
<dimen name="touch_to_fill_sheet_height_multiple_credentials">326dp</dimen>
<!-- Base height (282dp) plus bottom padding (16dp) -->
<dimen name="touch_to_fill_sheet_height_single_credential">298dp</dimen>
<!-- Base height (282dp) + confirm button with paddings (48+2*16dp) -->
<dimen name="touch_to_fill_sheet_height_single_credential_with_button">
362dp
</dimen>
</resources>
......@@ -8,6 +8,7 @@ import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.Cr
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FAVICON;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.FORMATTED_ORIGIN;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FIELD_TRIAL_PARAM_SHOW_CONFIRMATION_BUTTON;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.FORMATTED_URL;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.ORIGIN_SECURE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ON_CLICK_MANAGE;
......@@ -17,6 +18,7 @@ import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.VI
import androidx.annotation.Px;
import org.chromium.base.metrics.RecordHistogram;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillComponent.UserAction;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties;
......@@ -57,7 +59,6 @@ class TouchToFillMediator {
void showCredentials(String url, boolean isOriginSecure, List<Credential> credentials) {
assert credentials != null;
mModel.set(ON_CLICK_MANAGE, this::onManagePasswordSelected);
mModel.set(VISIBLE, true);
ListModel<ListItem> sheetItems = mModel.get(SHEET_ITEMS);
sheetItems.clear();
......@@ -71,18 +72,16 @@ class TouchToFillMediator {
mCredentials = credentials;
for (Credential credential : credentials) {
PropertyModel propertyModel =
new PropertyModel.Builder(CredentialProperties.ALL_KEYS)
.with(CREDENTIAL, credential)
.with(FORMATTED_ORIGIN,
UrlFormatter.formatUrlForDisplayOmitScheme(
credential.getOriginUrl()))
.with(ON_CLICK_LISTENER, this::onSelectedCredential)
.build();
sheetItems.add(new ListItem(TouchToFillProperties.ItemType.CREDENTIAL, propertyModel));
final PropertyModel model = createCredentialModel(credential);
sheetItems.add(new ListItem(TouchToFillProperties.ItemType.CREDENTIAL, model));
mDelegate.fetchFavicon(credential.getOriginUrl(), url, mDesiredFaviconSize,
(bitmap) -> propertyModel.set(FAVICON, bitmap));
(bitmap) -> model.set(FAVICON, bitmap));
if (shouldCreateConfirmationButton(credentials)) {
sheetItems.add(new ListItem(TouchToFillProperties.ItemType.FILL_BUTTON,
createCredentialModel(credential)));
}
}
mModel.set(VISIBLE, true);
}
private void onSelectedCredential(Credential credential) {
......@@ -116,4 +115,24 @@ class TouchToFillMediator {
UserAction.SELECT_MANAGE_PASSWORDS, UserAction.MAX_VALUE + 1);
mDelegate.onManagePasswordsSelected();
}
/**
* @param credentials The available credentials. Show the confirmation for a lone credential.
* @return True if a confirmation button should be shown at the end of the bottom sheet.
*/
private boolean shouldCreateConfirmationButton(List<Credential> credentials) {
return credentials.size() == 1
&& ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.TOUCH_TO_FILL_ANDROID,
FIELD_TRIAL_PARAM_SHOW_CONFIRMATION_BUTTON, false);
}
private PropertyModel createCredentialModel(Credential credential) {
return new PropertyModel.Builder(CredentialProperties.ALL_KEYS)
.with(CREDENTIAL, credential)
.with(ON_CLICK_LISTENER, this::onSelectedCredential)
.with(FORMATTED_ORIGIN,
UrlFormatter.formatUrlForDisplayOmitScheme(credential.getOriginUrl()))
.build();
}
}
......@@ -22,6 +22,8 @@ import java.lang.annotation.RetentionPolicy;
* Properties defined here reflect the visible state of the TouchToFill-components.
*/
class TouchToFillProperties {
static final String FIELD_TRIAL_PARAM_SHOW_CONFIRMATION_BUTTON = "show_confirmation_button";
static final PropertyModel.WritableBooleanPropertyKey VISIBLE =
new PropertyModel.WritableBooleanPropertyKey("visible");
static final PropertyModel
......@@ -73,7 +75,7 @@ class TouchToFillProperties {
private HeaderProperties() {}
}
@IntDef({ItemType.HEADER, ItemType.CREDENTIAL})
@IntDef({ItemType.HEADER, ItemType.CREDENTIAL, ItemType.FILL_BUTTON})
@Retention(RetentionPolicy.SOURCE)
@interface ItemType {
/**
......@@ -85,6 +87,11 @@ class TouchToFillProperties {
* A section containing a user's name and password.
*/
int CREDENTIAL = 2;
/**
* The fill button at the end of the sheet that filling more obvious for one suggestion.
*/
int FILL_BUTTON = 3;
}
/**
......
......@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.touch_to_fill;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FIELD_TRIAL_PARAM_SHOW_CONFIRMATION_BUTTON;
import android.content.Context;
import android.support.annotation.DimenRes;
import android.support.annotation.Nullable;
......@@ -14,6 +16,7 @@ import android.view.View;
import android.widget.LinearLayout;
import org.chromium.base.Callback;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetContent;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetController;
import org.chromium.chrome.browser.widget.bottomsheet.BottomSheetObserver;
......@@ -176,9 +179,16 @@ class TouchToFillView implements BottomSheetContent {
// TODO(crbug.com/1009331): This should add up the height of all items up to the 2nd credential.
private @DimenRes int getDesiredSheetHeight() {
if (mSheetItemListView.getAdapter() != null
&& mSheetItemListView.getAdapter().getItemCount() > 2) {
&& mSheetItemListView.getAdapter().getItemCount() > 2
&& mSheetItemListView.getAdapter().getItemViewType(2)
== TouchToFillProperties.ItemType.CREDENTIAL) {
return R.dimen.touch_to_fill_sheet_height_multiple_credentials;
}
if (ChromeFeatureList.getFieldTrialParamByFeatureAsBoolean(
ChromeFeatureList.TOUCH_TO_FILL_ANDROID,
FIELD_TRIAL_PARAM_SHOW_CONFIRMATION_BUTTON, false)) {
return R.dimen.touch_to_fill_sheet_height_single_credential_with_button;
}
return R.dimen.touch_to_fill_sheet_height_single_credential;
}
}
......@@ -74,6 +74,9 @@ class TouchToFillViewBinder {
case ItemType.CREDENTIAL:
return new TouchToFillViewHolder(parent, R.layout.touch_to_fill_credential_item,
TouchToFillViewBinder::bindCredentialView);
case ItemType.FILL_BUTTON:
return new TouchToFillViewHolder(parent, R.layout.touch_to_fill_fill_button,
TouchToFillViewBinder::bindFillButtonView);
}
assert false : "Cannot create view for ItemType: " + itemType;
return null;
......@@ -98,7 +101,7 @@ class TouchToFillViewBinder {
* @param propertyKey The key of the property to be bound
*/
private static void bindCredentialView(
PropertyModel model, ViewGroup view, PropertyKey propertyKey) {
PropertyModel model, View view, PropertyKey propertyKey) {
Credential credential = model.get(CREDENTIAL);
if (propertyKey == FAVICON) {
ImageView imageView = view.findViewById(R.id.favicon);
......@@ -131,20 +134,40 @@ class TouchToFillViewBinder {
}
}
/**
* Called whenever a fill button for a single credential is bound to this view holder.
* @param model The model containing the data for the view
* @param view The view to be bound
* @param propertyKey The key of the property to be bound
*/
private static void bindFillButtonView(
PropertyModel model, View view, PropertyKey propertyKey) {
Credential credential = model.get(CREDENTIAL);
if (propertyKey == ON_CLICK_LISTENER) {
view.setOnClickListener(
clickedView -> { model.get(ON_CLICK_LISTENER).onResult(credential); });
} else if (propertyKey == FAVICON || propertyKey == FORMATTED_ORIGIN
|| propertyKey == CREDENTIAL) {
// The button appearance is static.
} else {
assert false : "Unhandled update to property:" + propertyKey;
}
}
/**
* Called whenever a property in the given model changes. It updates the given view accordingly.
* @param model The observed {@link PropertyModel}. Its data need to be reflected in the view.
* @param viewGroup The {@link ViewGroup} containing the header to update.
* @param view The {@link View} of the header to update.
* @param key The {@link PropertyKey} which changed.
*/
private static void bindHeaderView(PropertyModel model, ViewGroup viewGroup, PropertyKey key) {
private static void bindHeaderView(PropertyModel model, View view, PropertyKey key) {
if (key == FORMATTED_URL || key == ORIGIN_SECURE) {
TextView sheetSubtitleText = viewGroup.findViewById(R.id.touch_to_fill_sheet_subtitle);
TextView sheetSubtitleText = view.findViewById(R.id.touch_to_fill_sheet_subtitle);
if (model.get(ORIGIN_SECURE)) {
sheetSubtitleText.setText(model.get(FORMATTED_URL));
} else {
sheetSubtitleText.setText(
String.format(viewGroup.getContext().getString(
String.format(view.getContext().getString(
R.string.touch_to_fill_sheet_subtitle_not_secure),
model.get(FORMATTED_URL)));
}
......
......@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.touch_to_fill;
import android.support.annotation.LayoutRes;
import android.support.v7.widget.RecyclerView;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import org.chromium.ui.modelutil.PropertyKey;
......@@ -15,10 +16,10 @@ import org.chromium.ui.modelutil.PropertyModelChangeProcessor;
import org.chromium.ui.modelutil.PropertyModelChangeProcessor.ViewBinder;
class TouchToFillViewHolder extends RecyclerView.ViewHolder {
final ViewBinder<PropertyModel, ViewGroup, PropertyKey> mViewBinder;
private final ViewBinder<PropertyModel, View, PropertyKey> mViewBinder;
TouchToFillViewHolder(ViewGroup parent, @LayoutRes int layout,
ViewBinder<PropertyModel, ViewGroup, PropertyKey> viewBinder) {
ViewBinder<PropertyModel, View, PropertyKey> viewBinder) {
super(LayoutInflater.from(parent.getContext()).inflate(layout, parent, false));
mViewBinder = viewBinder;
}
......@@ -30,6 +31,6 @@ class TouchToFillViewHolder extends RecyclerView.ViewHolder {
* @param model The {@link PropertyModel} whose data needs to be displayed.
*/
void setupModelChangeProcessor(PropertyModel model) {
PropertyModelChangeProcessor.create(model, (ViewGroup) itemView, mViewBinder, true);
PropertyModelChangeProcessor.create(model, itemView, mViewBinder, true);
}
}
\ No newline at end of file
......@@ -128,6 +128,9 @@
<message name="IDS_MANAGE_PASSWORDS" desc="Title of the button at the end of a touch to fill sheet that will open the password preferences when tapped.">
Manage Passwords
</message>
<message name="IDS_TOUCH_TO_FILL_CONFIRM" desc="Title of the button that confirms filling the only available set of credentials.">
Use Password
</message>
</messages>
</release>
</grit>
......@@ -191,6 +191,26 @@ public class TouchToFillViewTest {
waitForEvent(mCredentialCallback).onResult(eq(ANA));
}
@Test
@MediumTest
public void testSingleCredentialHasClickableButton() {
TestThreadUtils.runOnUiThreadBlocking(() -> {
mModel.get(SHEET_ITEMS)
.addAll(asList(
buildSheetItem(TouchToFillProperties.ItemType.CREDENTIAL, ANA, null),
buildConfirmationButton(ANA)));
mModel.set(VISIBLE, true);
});
pollUiThread(() -> getBottomSheetState() == BottomSheetController.SheetState.HALF);
assertNotNull(getCredentials().getChildAt(0));
assertNotNull(getCredentials().getChildAt(1));
TouchCommon.singleClickView(getCredentials().getChildAt(1));
waitForEvent(mCredentialCallback).onResult(eq(ANA));
}
@Test
@MediumTest
public void testManagePasswordsIsClickable() {
......@@ -258,10 +278,22 @@ public class TouchToFillViewTest {
}
private MVCListAdapter.ListItem buildCredentialItem(Credential credential) {
return new MVCListAdapter.ListItem(TouchToFillProperties.ItemType.CREDENTIAL,
return buildSheetItem(
TouchToFillProperties.ItemType.CREDENTIAL, credential, mCredentialCallback);
}
private MVCListAdapter.ListItem buildConfirmationButton(Credential credential) {
return buildSheetItem(
TouchToFillProperties.ItemType.FILL_BUTTON, credential, mCredentialCallback);
}
private static MVCListAdapter.ListItem buildSheetItem(
@TouchToFillProperties.ItemType int itemType, Credential credential,
Callback<Credential> callback) {
return new MVCListAdapter.ListItem(itemType,
new PropertyModel.Builder(TouchToFillProperties.CredentialProperties.ALL_KEYS)
.with(CREDENTIAL, credential)
.with(ON_CLICK_LISTENER, mCredentialCallback)
.with(ON_CLICK_LISTENER, callback)
.with(FORMATTED_ORIGIN, credential.getOriginUrl())
.build());
}
......
......@@ -27,6 +27,7 @@ import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.SH
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.VISIBLE;
import android.graphics.Bitmap;
import android.support.test.espresso.core.deps.guava.collect.ImmutableMap;
import androidx.annotation.Px;
......@@ -46,6 +47,7 @@ import org.chromium.base.metrics.RecordHistogramJni;
import org.chromium.base.metrics.test.ShadowRecordHistogram;
import org.chromium.base.test.BaseRobolectricTestRunner;
import org.chromium.base.test.util.JniMocker;
import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillComponent.UserAction;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ItemType;
import org.chromium.chrome.browser.touch_to_fill.data.Credential;
......@@ -97,6 +99,8 @@ public class TouchToFillControllerTest {
@Before
public void setUp() {
ShadowRecordHistogram.reset();
ChromeFeatureList.setTestFeatures(
ImmutableMap.of(ChromeFeatureList.TOUCH_TO_FILL_ANDROID, true));
MockitoAnnotations.initMocks(this);
mJniMocker.mock(UrlFormatterJni.TEST_HOOKS, mUrlFormatterJniMock);
mJniMocker.mock(RecordHistogramJni.TEST_HOOKS, mMockRecordHistogram);
......
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