Skip to content

Commit

Permalink
[M116][X-Device Restore] Scrolling back up with fling dismisses botto…
Browse files Browse the repository at this point in the history
…m sheet

Add a vertical scroll offset to prevent scroll flinging on the restore tabs review screen from dismissing the bottom sheet. Also addressed here is the ability to scroll on the promo screen if the bottom sheet size needs to be shrunken due to landscape/split screen, etc.

(cherry picked from commit fc55809)

Bug: 1458630
Change-Id: Id0e3ddce9ff2586dc0fcce2b4abb4b7f7366dd35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4642269
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Calder Kitagawa <ckitagawa@chromium.org>
Commit-Queue: Brandon Fong <bjfong@google.com>
Cr-Original-Commit-Position: refs/heads/main@{#1164234}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4661271
Reviewed-by: Xi Han <hanxi@chromium.org>
Cr-Commit-Position: refs/branch-heads/5845@{#263}
Cr-Branched-From: 5a5dff6-refs/heads/main@{#1160321}
  • Loading branch information
Brandon Fong authored and Chromium LUCI CQ committed Jun 30, 2023
1 parent aed5149 commit a165eee
Show file tree
Hide file tree
Showing 7 changed files with 200 additions and 104 deletions.
1 change: 1 addition & 0 deletions chrome/browser/recent_tabs/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ robolectric_library("junit") {
"//components/feature_engagement/public:public_java",
"//components/sync_device_info:sync_device_info_java",
"//third_party/android_deps:espresso_java",
"//third_party/androidx:androidx_recyclerview_recyclerview_java",
"//third_party/androidx:androidx_test_runner_java",
"//third_party/hamcrest:hamcrest_library_java",
"//third_party/junit:junit",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ found in the LICENSE file.
android:id="@+id/restore_tabs_toolbar_back_image_button"
android:layout_width="wrap_content"
android:layout_height="?attr/actionBarSize"
android:paddingLeft="20dp"
android:paddingRight="20dp"
android:paddingLeft="6dp"
android:paddingRight="6dp"
android:src="@drawable/ic_arrow_back_24dp"
app:tint="@macro/default_icon_color"
android:background="?attr/actionBarItemBackground"
Expand All @@ -35,6 +35,8 @@ found in the LICENSE file.
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_gravity="center_vertical"
android:paddingLeft="14dp"
android:paddingRight="14dp"
android:ellipsize="end"
android:singleLine="true"
style="@style/TextAppearance.Headline"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,112 +5,137 @@ Use of this source code is governed by a BSD-style license that can be
found in the LICENSE file.
-->

<LinearLayout
<!-- Notes:
The parent layout is not "useless", it acts as a buffer when using
include from the restore_tabs_bottom_sheet layout. This way, that
layout will not inherit the scroll view's ID and allow the promo
screen scroll offset logic to work properly.
Root frame merging is not really possible as the two layouts serve
different purposes, especially with the view flipper. Any actions
utilizing this view for property setting will end up returning null.
The scrollview allows the bottom sheet on this layout to scroll if
needed on multi-window, landscape, small screen, etc.
-->
<FrameLayout
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:app="http://schemas.android.com/apk/res-auto"
xmlns:tools="http://schemas.android.com/tools"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="vertical"
android:paddingLeft="@dimen/restore_tabs_sheet_padding"
android:paddingRight="@dimen/restore_tabs_sheet_padding">
<!-- Header section -->
<ImageView
android:layout_width="wrap_content"
android:layout_height="@dimen/restore_tabs_fre_logo_height"
android:layout_marginTop="24dp"
android:layout_gravity="center_horizontal"
android:importantForAccessibility="no"
app:srcCompat="@drawable/restore_tabs_fre_logo"/>
<LinearLayout
tools:ignore="UselessParent,MergeRootFrame">
<ScrollView
android:id="@+id/restore_tabs_promo_sheet_scrollview"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="24dp"
android:orientation="vertical"
android:focusable="true">
<TextView
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal"
android:textAppearance="@style/TextAppearance.Headline"
android:text="@string/restore_tabs_promo_sheet_title"/>
<TextView
android:id="@+id/restore_tabs_promo_sheet_subtitle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:gravity="center_horizontal"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary"/>
</LinearLayout>
<!-- Selected Device -->
<LinearLayout
android:id="@+id/restore_tabs_selected_device_view"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:layout_marginTop="24dp"
android:layout_marginBottom="8dp"
android:paddingTop="@dimen/restore_tabs_promo_sheet_device_card_padding"
android:paddingBottom="@dimen/restore_tabs_promo_sheet_device_card_padding"
android:background="@drawable/restore_tabs_single_item_background"
android:orientation="horizontal">
<ImageView
android:id="@+id/restore_tabs_promo_sheet_device_icon"
android:layout_width="@dimen/restore_tabs_device_icon"
android:layout_height="@dimen/restore_tabs_device_icon"
app:tint="@macro/default_icon_color"
android:background="@null"
android:layout_marginStart="@dimen/restore_tabs_device_icon_margin_horizontal"
android:layout_marginEnd="@dimen/restore_tabs_device_icon_margin_horizontal"
android:layout_gravity="center_vertical"
android:importantForAccessibility="no" />
<LinearLayout
android:layout_height="wrap_content"
android:layout_width="0dp"
android:layout_weight="1"
android:orientation="vertical">
<TextView
android:id="@+id/restore_tabs_promo_sheet_device_name"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="@style/TextAppearance.TextMedium.Primary"
android:textAlignment="viewStart" />
<TextView
android:id="@+id/restore_tabs_promo_sheet_session_info"
android:scrollbars="none">
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="@style/TextAppearance.TextSmall.Secondary"
android:textAlignment="viewStart" />
</LinearLayout>
<ImageView
android:id="@+id/restore_tabs_expand_icon_device_selection"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/restore_tabs_promo_sheet_expand_option_icon_margin_start"
android:layout_marginEnd="@dimen/restore_tabs_promo_sheet_expand_option_icon_margin_end"
android:layout_gravity="center_vertical"
android:importantForAccessibility="no"
android:contentDescription="@string/restore_tabs_promo_sheet_expand_icon_device_info_description"
app:tint="@color/default_icon_color_secondary_tint_list" />
</LinearLayout>
<!-- Action buttons section -->
<org.chromium.ui.widget.ButtonCompat
android:id="@+id/restore_tabs_button_open_tabs"
style="@style/FilledButton.Flat"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:ellipsize="end"
android:gravity="center"
android:singleLine="true"/>
<org.chromium.ui.widget.ButtonCompat
android:id="@+id/restore_tabs_button_review_tabs"
style="@style/TextButton"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="@dimen/restore_tabs_bottom_sheet_padding"
android:ellipsize="end"
android:gravity="center"
android:singleLine="true"
android:text="@string/restore_tabs_promo_sheet_review_tabs" />
</LinearLayout>
android:orientation="vertical"
android:paddingLeft="@dimen/restore_tabs_sheet_padding"
android:paddingRight="@dimen/restore_tabs_sheet_padding">
<ImageView
android:layout_width="wrap_content"
android:layout_height="@dimen/restore_tabs_fre_logo_height"
android:layout_marginTop="24dp"
android:layout_gravity="center_horizontal"
android:importantForAccessibility="no"
app:srcCompat="@drawable/restore_tabs_fre_logo"/>
<LinearLayout
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="24dp"
android:orientation="vertical"
android:focusable="true">
<TextView
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:gravity="center_horizontal"
android:textAppearance="@style/TextAppearance.Headline"
android:text="@string/restore_tabs_promo_sheet_title"/>
<TextView
android:id="@+id/restore_tabs_promo_sheet_subtitle"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:gravity="center_horizontal"
android:textAppearance="@style/TextAppearance.TextMedium.Secondary"/>
</LinearLayout>
<!-- Selected Device -->
<LinearLayout
android:id="@+id/restore_tabs_selected_device_view"
android:layout_height="wrap_content"
android:layout_width="match_parent"
android:layout_marginTop="24dp"
android:layout_marginBottom="8dp"
android:paddingTop="@dimen/restore_tabs_promo_sheet_device_card_padding"
android:paddingBottom="@dimen/restore_tabs_promo_sheet_device_card_padding"
android:background="@drawable/restore_tabs_single_item_background"
android:orientation="horizontal">
<ImageView
android:id="@+id/restore_tabs_promo_sheet_device_icon"
android:layout_width="@dimen/restore_tabs_device_icon"
android:layout_height="@dimen/restore_tabs_device_icon"
app:tint="@macro/default_icon_color"
android:background="@null"
android:layout_marginStart="@dimen/restore_tabs_device_icon_margin_horizontal"
android:layout_marginEnd="@dimen/restore_tabs_device_icon_margin_horizontal"
android:layout_gravity="center_vertical"
android:importantForAccessibility="no" />
<LinearLayout
android:layout_height="wrap_content"
android:layout_width="0dp"
android:layout_weight="1"
android:orientation="vertical">
<TextView
android:id="@+id/restore_tabs_promo_sheet_device_name"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="@style/TextAppearance.TextMedium.Primary"
android:textAlignment="viewStart" />
<TextView
android:id="@+id/restore_tabs_promo_sheet_session_info"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textAppearance="@style/TextAppearance.TextSmall.Secondary"
android:textAlignment="viewStart" />
</LinearLayout>
<ImageView
android:id="@+id/restore_tabs_expand_icon_device_selection"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/restore_tabs_promo_sheet_expand_option_icon_margin_start"
android:layout_marginEnd="@dimen/restore_tabs_promo_sheet_expand_option_icon_margin_end"
android:layout_gravity="center_vertical"
android:importantForAccessibility="no"
android:contentDescription="@string/restore_tabs_promo_sheet_expand_icon_device_info_description"
app:tint="@color/default_icon_color_secondary_tint_list" />
</LinearLayout>
<!-- Action buttons section -->
<org.chromium.ui.widget.ButtonCompat
android:id="@+id/restore_tabs_button_open_tabs"
style="@style/FilledButton.Flat"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="8dp"
android:ellipsize="end"
android:gravity="center"
android:singleLine="true"/>
<org.chromium.ui.widget.ButtonCompat
android:id="@+id/restore_tabs_button_review_tabs"
style="@style/TextButton"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="4dp"
android:layout_marginBottom="@dimen/restore_tabs_bottom_sheet_padding"
android:ellipsize="end"
android:gravity="center"
android:singleLine="true"
android:text="@string/restore_tabs_promo_sheet_review_tabs" />
</LinearLayout>
</ScrollView>
</FrameLayout>
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ found in the LICENSE file.

<resources>
<!-- General dimensions. -->
<dimen name="restore_tabs_sheet_padding">10dp</dimen>
<dimen name="restore_tabs_sheet_padding">24dp</dimen>
<dimen name="restore_tabs_sheet_padding_content">24dp</dimen>
<dimen name="restore_tabs_device_icon">20dp</dimen>
<dimen name="restore_tabs_device_icon_margin_horizontal">18dp</dimen>
<dimen name="restore_tabs_tab_icon_margin_horizontal">18dp</dimen>
<dimen name="restore_tabs_fre_logo_height">100dp</dimen>
<dimen name="restore_tabs_inner_corner_radius">4dp</dimen>
<dimen name="restore_tabs_outer_corner_radius">16dp</dimen>
<dimen name="restore_tabs_bottom_sheet_padding">24dp</dimen>
<dimen name="restore_tabs_bottom_sheet_padding">16dp</dimen>

<!-- Home sheet dimensions. -->
<dimen name="restore_tabs_promo_sheet_device_card_padding">16dp</dimen>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@
import static org.chromium.chrome.browser.recent_tabs.RestoreTabsProperties.ScreenType.UNINITIALIZED;

import android.view.View;
import android.widget.ScrollView;

import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import androidx.recyclerview.widget.RecyclerView;

import org.chromium.base.supplier.ObservableSupplierImpl;
import org.chromium.chrome.browser.recent_tabs.RestoreTabsMetricsHelper.RestoreTabsOnFREBackPressType;
Expand All @@ -33,12 +36,16 @@ public class RestoreTabsPromoSheetContent implements BottomSheetContent {
private final BottomSheetObserver mBottomSheetOpenedObserver;
private final ObservableSupplierImpl<Boolean> mBackPressStateChangedSupplier =
new ObservableSupplierImpl<>();
private ScrollView mScrollView;
private RecyclerView mRecyclerView;

public RestoreTabsPromoSheetContent(
View contentView, PropertyModel model, BottomSheetController bottomSheetController) {
mContentView = contentView;
mModel = model;
mBottomSheetController = bottomSheetController;
mScrollView = mContentView.findViewById(R.id.restore_tabs_promo_sheet_scrollview);
mRecyclerView = mContentView.findViewById(R.id.restore_tabs_detail_screen_recycler_view);

mBottomSheetOpenedObserver = new EmptyBottomSheetObserver() {
@Override
Expand Down Expand Up @@ -68,8 +75,28 @@ public View getToolbarView() {
return null;
}

/**
* The vertical scroll offset of the recycler view's list containing the user's devices
* or the currently selected device's tab items. The offset prevents scroll flinging from
* dismissing the sheet.
*/
@Override
public int getVerticalScrollOffset() {
int currentScreen = mModel.get(RestoreTabsProperties.CURRENT_SCREEN);

if (currentScreen == HOME_SCREEN) {
if (mScrollView != null) {
// Calculate the scroll position of the scrollview and make sure it is
// non-zero, otherwise allows swipe to dismiss on the bottom sheet.
return mScrollView.getScrollY();
}
} else if (currentScreen == DEVICE_SCREEN || currentScreen == REVIEW_TABS_SCREEN) {
// Get the first item in the recycler view to make sure it is scrolled off-screen
// (has non-zero value) otherwise allow swipe to dismiss on the bottom sheet.
View v = mRecyclerView.getChildAt(0);
return v == null ? 0 : -(v.getTop() - mRecyclerView.getPaddingTop());
}

return 0;
}

Expand Down Expand Up @@ -160,4 +187,14 @@ private void backPressOnCurrentScreen() {
RestoreTabsOnFREBackPressType.SYSTEM_BACKPRESS);
}
}

@VisibleForTesting
void setRecyclerViewForTesting(RecyclerView recyclerView) {
mRecyclerView = recyclerView;
}

@VisibleForTesting
void setScrollViewForTesting(ScrollView scrollView) {
mScrollView = scrollView;
}
}

0 comments on commit a165eee

Please sign in to comment.