Skip to content

Commit

Permalink
[TTFCC] Making the entire sheet scrollable
Browse files Browse the repository at this point in the history
This change applies for the bottom sheets for payments and passwords.
For a relatively small amount of cards/passwords, the UI will still look
the same. The difference is that the footer actions (scanning the card,
going to payment settings, managing passwords) won't be fixed on the
bottom of the bottom sheet in full height, but instead only reachable by
scrolling down.

Changes:
- Separating the footers into their own xmls.
- Sharing the main sheet layout between passwords and payments.
- Moving the properties related to the footers into FooterProperties.
- Updating the way of binding the footer Views.
- Simplifying the sheet height calculation because the footer height is
calculated the same way as for the other list items.
- Updating the tests.
- Updating the background selection.

Screenshots:
2 cards, full height:
https://screenshot.googleplex.com/8UuFmJiD66zVr54
2 cards: half height:
https://screenshot.googleplex.com/4ydyBi82ZudeDTj
many cards, full height initial:
https://screenshot.googleplex.com/9BphTpcgVQ9Eurn
many cards, full height scrolled:
https://screenshot.googleplex.com/7CyxM6STQQYSnwg

3 passwords, half state:
https://screenshot.googleplex.com/5P5PkU3oio5bct5
4 passwords half state:
https://screenshot.googleplex.com/Bd8Y2yFGdXGWkew

There is a TODO for fixing the case of 4 or more items.
I'll fix it separately.

Bug: 1247698
Change-Id: I108ac27aebec825b47129d22a50aef1b118d78f1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4228007
Reviewed-by: Friedrich Horschig <fhorschig@chromium.org>
Commit-Queue: Ivana Žužić <izuzic@google.com>
Cr-Commit-Position: refs/heads/main@{#1107758}
  • Loading branch information
Ivana Žužić authored and Chromium LUCI CQ committed Feb 21, 2023
1 parent 80a6385 commit ae94646
Show file tree
Hide file tree
Showing 33 changed files with 476 additions and 373 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.chromium.base.test.util.Matchers;
import org.chromium.chrome.browser.flags.ChromeFeatureList;
import org.chromium.chrome.browser.flags.ChromeSwitches;
import org.chromium.chrome.browser.touch_to_fill.R;
import org.chromium.chrome.browser.touch_to_fill.common.R;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.chrome.test.ChromeTabbedActivityTestRule;
import org.chromium.chrome.test.util.ChromeTabUtils;
Expand Down Expand Up @@ -229,7 +229,6 @@ private void waitForPmParserAnnotation(WebContents webContents, String nodeID) {
}

private RecyclerView getCredentials() {
RecyclerView r = mActivityTestRule.getActivity().findViewById(R.id.sheet_item_list);
return mActivityTestRule.getActivity().findViewById(R.id.sheet_item_list);
}
}
1 change: 1 addition & 0 deletions chrome/browser/touch_to_fill/android/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ android_library("test_java") {
"//chrome/browser/flags:java",
"//chrome/browser/touch_to_fill/android/internal:java",
"//chrome/browser/touch_to_fill/android/internal:resource_provider_public_impl_java",
"//chrome/browser/touch_to_fill/common/android:java_resources",
"//chrome/browser/ui/android/night_mode:night_mode_java_test_support",
"//chrome/browser/util:java",
"//chrome/test/android:chrome_java_integration_test_support",
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/touch_to_fill/android/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,9 @@ android_resources("java_resources") {
"java/res/layout/touch_to_fill_credential_item.xml",
"java/res/layout/touch_to_fill_credential_item_modern.xml",
"java/res/layout/touch_to_fill_fill_button.xml",
"java/res/layout/touch_to_fill_footer_item.xml",
"java/res/layout/touch_to_fill_header_item.xml",
"java/res/layout/touch_to_fill_header_item_modern.xml",
"java/res/layout/touch_to_fill_sheet.xml",
"java/res/layout/touch_to_fill_webauthn_credential_item.xml",
"java/res/layout/touch_to_fill_webauthn_credential_item_modern.xml",
"java/res/values/dimens.xml",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ found in the LICENSE file.
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginBottom="8dp"
android:layout_marginHorizontal="8dp"
android:minHeight="72dp"
android:gravity="center_vertical"
android:orientation="horizontal"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ found in the LICENSE file.
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="2dp"
android:layout_marginBottom="2dp"
android:layout_marginHorizontal="8dp"
android:minHeight="48dp"
android:gravity="center"
android:background="@drawable/touch_to_fill_credential_background"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?xml version="1.0" encoding="utf-8"?>
<!--
Copyright 2023 The Chromium Authors
Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->

<LinearLayout
xmlns:android="http://schemas.android.com/apk/res/android"
android:id="@+id/touch_to_fill_footer"
android:layout_width="match_parent"
android:orientation="vertical"
android:layout_height="wrap_content"
android:layout_alignParentBottom="true"
android:layout_marginBottom="@dimen/ttf_buttons_vertical_margin"
android:layout_marginTop="@dimen/ttf_sheet_padding">

<!-- Divider -->
<View style="@style/HorizontalDivider"
android:layout_width="match_parent"
android:layout_height="@dimen/divider_height"
android:layout_marginBottom="@dimen/ttf_buttons_vertical_margin"/>

<TextView
android:id="@+id/touch_to_fill_sheet_manage_passwords"
android:layout_width="match_parent"
android:layout_height="@dimen/ttf_buttons_height"
android:paddingHorizontal="@dimen/ttf_sheet_padding"
android:minHeight="48dp"
android:gravity="center_vertical|start"
android:textAppearance="@style/TextAppearance.TextLarge.Primary"
android:background="?android:attr/selectableItemBackground"
android:textDirection="locale"
android:textAlignment="viewStart"/>
</LinearLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ found in the LICENSE file.
<dimen name="touch_to_fill_favicon_size">24dp</dimen>
<dimen name="touch_to_fill_favicon_size_modern">32dp</dimen>
<dimen name="touch_to_fill_sheet_margin">16dp</dimen>
<dimen name="touch_to_fill_sheet_margin_modern">24dp</dimen>
<dimen name="touch_to_fill_sheet_margin_modern">16dp</dimen>

<!-- Depending on the experiments that are active we might show a call to
action button or branding message to the users, at which point we need
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
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.CredentialProperties.SHOW_SUBMIT_BUTTON;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FooterProperties.MANAGE_BUTTON_TEXT;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FooterProperties.ON_CLICK_MANAGE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.FORMATTED_URL;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.IMAGE_DRAWABLE_ID;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.ORIGIN_SECURE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.SHOW_SUBMIT_SUBTITLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.TITLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.MANAGE_BUTTON_TEXT;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ON_CLICK_MANAGE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.VISIBLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.WebAuthnCredentialProperties.ON_WEBAUTHN_CLICK_LISTENER;
Expand All @@ -32,6 +32,7 @@
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.FaviconOrFallback;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FooterProperties;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties;
import org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.WebAuthnCredentialProperties;
import org.chromium.chrome.browser.touch_to_fill.data.Credential;
Expand Down Expand Up @@ -83,8 +84,6 @@ void showCredentials(GURL url, boolean isOriginSecure,
List<WebAuthnCredential> webAuthnCredentials, List<Credential> credentials,
boolean triggerSubmission) {
assert credentials != null;
mModel.set(ON_CLICK_MANAGE, this::onManagePasswordSelected);
mModel.set(MANAGE_BUTTON_TEXT, getManageButtonText(credentials, webAuthnCredentials));

TouchToFillResourceProvider resourceProvider = new TouchToFillResourceProviderImpl();

Expand Down Expand Up @@ -122,6 +121,13 @@ void showCredentials(GURL url, boolean isOriginSecure,
requestIconOrFallbackImage(model, url);
}

sheetItems.add(new ListItem(TouchToFillProperties.ItemType.FOOTER,
new PropertyModel.Builder(FooterProperties.ALL_KEYS)
.with(ON_CLICK_MANAGE, this::onManagePasswordSelected)
.with(MANAGE_BUTTON_TEXT,
getManageButtonText(credentials, webAuthnCredentials))
.build()));

mModel.set(VISIBLE, true);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,8 @@ class TouchToFillProperties {
new PropertyModel.ReadableObjectPropertyKey<>("sheet_items");
static final PropertyModel.ReadableObjectPropertyKey<Callback<Integer>> DISMISS_HANDLER =
new PropertyModel.ReadableObjectPropertyKey<>("dismiss_handler");
static final PropertyModel.WritableObjectPropertyKey<Runnable> ON_CLICK_MANAGE =
new PropertyModel.WritableObjectPropertyKey<>("on_click_manage");
static final PropertyModel.WritableObjectPropertyKey<String> MANAGE_BUTTON_TEXT =
new PropertyModel.WritableObjectPropertyKey<>("manage_button_text");
static PropertyModel createDefaultModel(Callback<Integer> handler) {
return new PropertyModel
.Builder(VISIBLE, SHEET_ITEMS, DISMISS_HANDLER, ON_CLICK_MANAGE, MANAGE_BUTTON_TEXT)
return new PropertyModel.Builder(VISIBLE, SHEET_ITEMS, DISMISS_HANDLER)
.with(VISIBLE, false)
.with(SHEET_ITEMS, new ListModel<>())
.with(DISMISS_HANDLER, handler)
Expand Down Expand Up @@ -129,8 +124,24 @@ static class HeaderProperties {
private HeaderProperties() {}
}

/**
* Properties defined here reflect the visible state of the footer in the TouchToFill sheet for
* payments.
*/
static class FooterProperties {
// TODO(crbug.com/1247698): Use ReadableBooleanPropertyKey.
static final PropertyModel.WritableObjectPropertyKey<Runnable> ON_CLICK_MANAGE =
new PropertyModel.WritableObjectPropertyKey<>("on_click_manage");
static final PropertyModel.WritableObjectPropertyKey<String> MANAGE_BUTTON_TEXT =
new PropertyModel.WritableObjectPropertyKey<>("manage_button_text");

static final PropertyKey[] ALL_KEYS = {ON_CLICK_MANAGE, MANAGE_BUTTON_TEXT};

private FooterProperties() {}
}

@IntDef({ItemType.HEADER, ItemType.CREDENTIAL, ItemType.WEBAUTHN_CREDENTIAL,
ItemType.FILL_BUTTON})
ItemType.FILL_BUTTON, ItemType.FOOTER})
@Retention(RetentionPolicy.SOURCE)
@interface ItemType {
/**
Expand All @@ -152,6 +163,11 @@ private HeaderProperties() {}
* The fill button at the end of the sheet that filling more obvious for one suggestion.
*/
int FILL_BUTTON = 4;

/**
* A footer section containing additional actions.
*/
int FOOTER = 5;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import android.view.LayoutInflater;
import android.view.View;
import android.widget.RelativeLayout;
import android.widget.TextView;

import androidx.annotation.Px;
import androidx.recyclerview.widget.RecyclerView;
Expand Down Expand Up @@ -48,6 +47,7 @@ protected boolean shouldSkipItemType(@ItemType int type) {
switch (type) {
case ItemType.HEADER: // Fallthrough.
case ItemType.FILL_BUTTON:
case ItemType.FOOTER:
return true;
case ItemType.CREDENTIAL: // Fallthrough.
case ItemType.WEBAUTHN_CREDENTIAL:
Expand All @@ -59,8 +59,10 @@ protected boolean shouldSkipItemType(@ItemType int type) {

@Override
protected boolean containsFillButton(RecyclerView parent) {
return parent.getAdapter().getItemViewType(parent.getAdapter().getItemCount() - 1)
== ItemType.FILL_BUTTON;
int itemCount = parent.getAdapter().getItemCount();
// The button will be above the footer if it's present.
return itemCount > 1
&& parent.getAdapter().getItemViewType(itemCount - 2) == ItemType.FILL_BUTTON;
}
}

Expand All @@ -82,17 +84,6 @@ protected boolean containsFillButton(RecyclerView parent) {
}
}

void setOnManagePasswordClick(Runnable runnable) {
getContentView()
.findViewById(R.id.touch_to_fill_sheet_manage_passwords)
.setOnClickListener((v) -> runnable.run());
}

void setManagePasswordText(String buttonText) {
TextView view = getContentView().findViewById(R.id.touch_to_fill_sheet_manage_passwords);
view.setText(buttonText);
}

@Override
public int getVerticalScrollOffset() {
return mSheetItemListView.computeVerticalScrollOffset();
Expand Down Expand Up @@ -124,8 +115,8 @@ protected View getHandlebar() {
}

@Override
protected View getFooter() {
return getContentView().findViewById(R.id.touch_to_fill_footer);
protected int getFooterId() {
return R.id.touch_to_fill_footer;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.ON_CLICK_LISTENER;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.CredentialProperties.SHOW_SUBMIT_BUTTON;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.DISMISS_HANDLER;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FooterProperties.MANAGE_BUTTON_TEXT;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.FooterProperties.ON_CLICK_MANAGE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.FORMATTED_URL;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.IMAGE_DRAWABLE_ID;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.ORIGIN_SECURE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.SHOW_SUBMIT_SUBTITLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.HeaderProperties.TITLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.MANAGE_BUTTON_TEXT;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.ON_CLICK_MANAGE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.SHEET_ITEMS;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.VISIBLE;
import static org.chromium.chrome.browser.touch_to_fill.TouchToFillProperties.WebAuthnCredentialProperties.ON_WEBAUTHN_CLICK_LISTENER;
Expand Down Expand Up @@ -69,10 +69,6 @@ static void bindTouchToFillView(
assert (model.get(DISMISS_HANDLER) != null);
model.get(DISMISS_HANDLER).onResult(BottomSheetController.StateChangeReason.NONE);
}
} else if (propertyKey == ON_CLICK_MANAGE) {
view.setOnManagePasswordClick(model.get(ON_CLICK_MANAGE));
} else if (propertyKey == MANAGE_BUTTON_TEXT) {
view.setManagePasswordText(model.get(MANAGE_BUTTON_TEXT));
} else if (propertyKey == SHEET_ITEMS) {
view.setSheetItemListAdapter(
new RecyclerViewAdapter<>(new SimpleRecyclerViewMcp<>(model.get(SHEET_ITEMS),
Expand Down Expand Up @@ -116,6 +112,9 @@ private static TouchToFillViewHolder createViewHolder(
? R.layout.touch_to_fill_fill_button_modern
: R.layout.touch_to_fill_fill_button,
TouchToFillViewBinder::bindFillButtonView);
case ItemType.FOOTER:
return new TouchToFillViewHolder(parent, R.layout.touch_to_fill_footer_item,
TouchToFillViewBinder::bindFooterView);
}
assert false : "Cannot create view for ItemType: " + itemType;
return null;
Expand Down Expand Up @@ -301,5 +300,24 @@ private static void bindHeaderView(PropertyModel model, View view, PropertyKey k
}
}

/**
* 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 view The {@link View} of the header to update.
* @param key The {@link PropertyKey} which changed.
*/
private static void bindFooterView(PropertyModel model, View view, PropertyKey key) {
if (key == ON_CLICK_MANAGE) {
view.findViewById(R.id.touch_to_fill_sheet_manage_passwords)
.setOnClickListener((v) -> model.get(ON_CLICK_MANAGE).run());
} else if (key == MANAGE_BUTTON_TEXT) {
TextView managePasswordsView =
view.findViewById(R.id.touch_to_fill_sheet_manage_passwords);
managePasswordsView.setText(model.get(MANAGE_BUTTON_TEXT));
} else {
assert false : "Unhandled update to property:" + key;
}
}

private TouchToFillViewBinder() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,8 @@ public int getSheetClosedAccessibilityStringId() {
}

private RecyclerView getCredentials() {
return mActivityTestRule.getActivity().findViewById(R.id.sheet_item_list);
return mActivityTestRule.getActivity().findViewById(
org.chromium.chrome.browser.touch_to_fill.common.R.id.sheet_item_list);
}

private TextView getManagePasswordsButton() {
Expand Down

0 comments on commit ae94646

Please sign in to comment.