diff options
13 files changed, 175 insertions, 34 deletions
diff --git a/res/flag(com.android.documentsui.flags.use_material3)/layout/drawer_layout.xml b/res/flag(com.android.documentsui.flags.use_material3)/layout/drawer_layout.xml index fc4a22bc6..aeb85442f 100644 --- a/res/flag(com.android.documentsui.flags.use_material3)/layout/drawer_layout.xml +++ b/res/flag(com.android.documentsui.flags.use_material3)/layout/drawer_layout.xml @@ -83,7 +83,8 @@ android:id="@+id/horizontal_breadcrumb" android:layout_width="match_parent" android:layout_height="wrap_content" - android:background="?attr/colorSurfaceBright" /> + android:background="?attr/colorSurfaceBright" + android:paddingHorizontal="@dimen/breadcrumb_padding_horizontal" /> <androidx.coordinatorlayout.widget.CoordinatorLayout android:id="@+id/container_save" diff --git a/res/flag(com.android.documentsui.flags.use_material3)/layout/fixed_layout.xml b/res/flag(com.android.documentsui.flags.use_material3)/layout/fixed_layout.xml index 5496f0d84..c2a06122a 100644 --- a/res/flag(com.android.documentsui.flags.use_material3)/layout/fixed_layout.xml +++ b/res/flag(com.android.documentsui.flags.use_material3)/layout/fixed_layout.xml @@ -115,7 +115,8 @@ <com.android.documentsui.HorizontalBreadcrumb android:id="@+id/horizontal_breadcrumb" android:layout_width="match_parent" - android:layout_height="wrap_content" /> + android:layout_height="wrap_content" + android:paddingHorizontal="@dimen/breadcrumb_padding_horizontal" /> </LinearLayout> diff --git a/res/flag(com.android.documentsui.flags.use_material3)/layout/nav_rail_layout.xml b/res/flag(com.android.documentsui.flags.use_material3)/layout/nav_rail_layout.xml index bd85b7dac..bfbb83c17 100644 --- a/res/flag(com.android.documentsui.flags.use_material3)/layout/nav_rail_layout.xml +++ b/res/flag(com.android.documentsui.flags.use_material3)/layout/nav_rail_layout.xml @@ -147,7 +147,8 @@ <com.android.documentsui.HorizontalBreadcrumb android:id="@+id/horizontal_breadcrumb" android:layout_width="match_parent" - android:layout_height="wrap_content" /> + android:layout_height="wrap_content" + android:paddingHorizontal="@dimen/breadcrumb_padding_horizontal" /> </LinearLayout> <androidx.coordinatorlayout.widget.CoordinatorLayout diff --git a/res/flag(com.android.documentsui.flags.use_material3)/layout/search_chip_row.xml b/res/flag(com.android.documentsui.flags.use_material3)/layout/search_chip_row.xml index 559ae3188..62a36f3e8 100644 --- a/res/flag(com.android.documentsui.flags.use_material3)/layout/search_chip_row.xml +++ b/res/flag(com.android.documentsui.flags.use_material3)/layout/search_chip_row.xml @@ -20,10 +20,21 @@ android:layout_height="wrap_content" android:scrollbars="none"> - <com.google.android.material.chip.ChipGroup - android:id="@+id/search_chip_group" + <!-- This additional FrameLayout layer is essential to make HorizontalScrollView work with the + marginHorizontal on the ChipGroup below, without this the scroll behavior is weird. + Alternatively we could use paddingHorizontal on the ChipGroup below to make it work with + HorizontalScrollView, but that cause a weird padding issue in RTL. + --> + <FrameLayout android:layout_width="match_parent" - android:layout_height="wrap_content" - android:paddingHorizontal="@dimen/main_container_padding_start" - android:paddingVertical="@dimen/search_chip_group_padding_vertical" /> + android:layout_height="wrap_content"> + + <com.google.android.material.chip.ChipGroup + android:id="@+id/search_chip_group" + android:layout_width="match_parent" + android:layout_height="wrap_content" + android:layout_marginHorizontal="@dimen/main_container_padding_start" + android:paddingVertical="@dimen/search_chip_group_padding_vertical" /> + + </FrameLayout> </HorizontalScrollView> diff --git a/res/flag(com.android.documentsui.flags.use_material3)/values/dimens.xml b/res/flag(com.android.documentsui.flags.use_material3)/values/dimens.xml index f9b9eddfd..872cf32ce 100644 --- a/res/flag(com.android.documentsui.flags.use_material3)/values/dimens.xml +++ b/res/flag(com.android.documentsui.flags.use_material3)/values/dimens.xml @@ -59,7 +59,12 @@ <dimen name="list_item_icon_padding">16dp</dimen> <dimen name="list_divider_inset">72dp</dimen> <!-- block end --> + <dimen name="breadcrumb_padding_horizontal">@dimen/space_small_3</dimen> + <!-- TODO(b/379776735): remove this after use_material3 flag is launched. --> <dimen name="breadcrumb_item_padding">8dp</dimen> + <dimen name="breadcrumb_item_padding_horizontal">12dp</dimen> + <dimen name="breadcrumb_item_padding_vertical">6dp</dimen> + <dimen name="breadcrumb_item_arrow_padding">@dimen/space_extra_small_2</dimen> <dimen name="breadcrumb_item_height">36dp</dimen> <dimen name="dir_elevation">8dp</dimen> <dimen name="drag_shadow_size">120dp</dimen> diff --git a/src/com/android/documentsui/HorizontalBreadcrumb.java b/src/com/android/documentsui/HorizontalBreadcrumb.java index cb25479b3..f47f464c0 100644 --- a/src/com/android/documentsui/HorizontalBreadcrumb.java +++ b/src/com/android/documentsui/HorizontalBreadcrumb.java @@ -16,6 +16,8 @@ package com.android.documentsui; +import static com.android.documentsui.util.FlagUtils.isUseMaterial3FlagEnabled; + import android.content.Context; import android.util.AttributeSet; import android.view.GestureDetector; @@ -183,8 +185,6 @@ public final class HorizontalBreadcrumb extends RecyclerView implements Breadcru @Override public void onBindViewHolder(BreadcrumbHolder holder, int position) { - final int padding = (int) holder.itemView.getResources() - .getDimension(R.dimen.breadcrumb_item_padding); final boolean isFirst = position == 0; // Note that when isFirst is true, there might not be a DocumentInfo on the stack as it // could be an error state screen accessible from the root info. @@ -193,8 +193,39 @@ public final class HorizontalBreadcrumb extends RecyclerView implements Breadcru holder.mTitle.setText( isFirst ? mEnv.getCurrentRoot().title : mState.stack.get(position).displayName); holder.mTitle.setEnabled(isLast); - holder.mTitle.setPadding(isFirst ? padding * 3 : padding, - padding, isLast ? padding * 2 : padding, padding); + if (isUseMaterial3FlagEnabled()) { + final int paddingHorizontal = + (int) + holder.itemView + .getResources() + .getDimension(R.dimen.breadcrumb_item_padding_horizontal); + final int paddingVertical = + (int) + holder.itemView + .getResources() + .getDimension(R.dimen.breadcrumb_item_padding_vertical); + final int arrowPadding = + (int) + holder.itemView + .getResources() + .getDimension(R.dimen.breadcrumb_item_arrow_padding); + holder.mTitle.setPadding( + paddingHorizontal, paddingVertical, paddingHorizontal, paddingVertical); + + ViewGroup.MarginLayoutParams params = + (ViewGroup.MarginLayoutParams) holder.mArrow.getLayoutParams(); + params.setMarginStart(arrowPadding); + params.setMarginEnd(arrowPadding); + holder.mArrow.setLayoutParams(params); + } else { + final int padding = (int) holder.itemView.getResources() + .getDimension(R.dimen.breadcrumb_item_padding); + holder.mTitle.setPadding( + isFirst ? padding * 3 : padding, + padding, + isLast ? padding * 2 : padding, + padding); + } holder.mArrow.setVisibility(isLast ? View.GONE : View.VISIBLE); holder.itemView.setOnKeyListener(mClickListener); diff --git a/src/com/android/documentsui/ProfileTabs.java b/src/com/android/documentsui/ProfileTabs.java index 5aacc22b0..74db6f4bd 100644 --- a/src/com/android/documentsui/ProfileTabs.java +++ b/src/com/android/documentsui/ProfileTabs.java @@ -157,9 +157,12 @@ public class ProfileTabs implements ProfileTabsAddons { int tabMarginSide = (int) mTabsContainer.getContext().getResources() .getDimension(R.dimen.profile_tab_margin_side); if (isUseMaterial3FlagEnabled()) { - // M3 uses the margin value as the right margin, except for the last child. + final boolean isRtl = mTabs.getLayoutDirection() == View.LAYOUT_DIRECTION_RTL; + // if use_material3 flag is ON, we uses the margin value as the right margin + // (left margin for RTL), except for the last child. if (i != mTabs.getTabCount() - 1) { - marginLayoutParams.setMargins(0, 0, tabMarginSide, 0); + marginLayoutParams.setMargins( + isRtl ? tabMarginSide : 0, 0, isRtl ? 0 : tabMarginSide, 0); } } else { marginLayoutParams.setMargins(tabMarginSide, 0, tabMarginSide, 0); diff --git a/src/com/android/documentsui/dirlist/DocumentsSwipeRefreshLayout.java b/src/com/android/documentsui/dirlist/DocumentsSwipeRefreshLayout.java index 838b1fa72..64409673e 100644 --- a/src/com/android/documentsui/dirlist/DocumentsSwipeRefreshLayout.java +++ b/src/com/android/documentsui/dirlist/DocumentsSwipeRefreshLayout.java @@ -22,13 +22,13 @@ import android.content.Context; import android.content.res.TypedArray; import android.util.AttributeSet; import android.util.Log; -import android.util.TypedValue; import android.view.MotionEvent; import androidx.annotation.ColorRes; import androidx.swiperefreshlayout.widget.SwipeRefreshLayout; import com.android.documentsui.R; +import com.android.documentsui.util.ColorUtils; /** * A {@link SwipeRefreshLayout} that does not intercept any touch events. This relies on its nested @@ -46,20 +46,12 @@ public class DocumentsSwipeRefreshLayout extends SwipeRefreshLayout { super(context, attrs); if (isUseMaterial3FlagEnabled()) { - TypedValue spinnerColor = new TypedValue(); - context.getTheme() - .resolveAttribute( - com.google.android.material.R.attr.colorOnPrimaryContainer, - spinnerColor, - true); - setColorSchemeResources(spinnerColor.resourceId); - TypedValue spinnerBackgroundColor = new TypedValue(); - context.getTheme() - .resolveAttribute( - com.google.android.material.R.attr.colorPrimaryContainer, - spinnerBackgroundColor, - true); - setProgressBackgroundColorSchemeResource(spinnerBackgroundColor.resourceId); + setColorSchemeColors( + ColorUtils.resolveMaterialColorAttribute( + context, com.google.android.material.R.attr.colorOnPrimaryContainer)); + setProgressBackgroundColorSchemeColor( + ColorUtils.resolveMaterialColorAttribute( + context, com.google.android.material.R.attr.colorPrimaryContainer)); } else { final int[] styledAttrs = {android.R.attr.colorAccent}; diff --git a/src/com/android/documentsui/dirlist/GridDocumentHolder.java b/src/com/android/documentsui/dirlist/GridDocumentHolder.java index 9523c331c..431ace5d9 100644 --- a/src/com/android/documentsui/dirlist/GridDocumentHolder.java +++ b/src/com/android/documentsui/dirlist/GridDocumentHolder.java @@ -299,8 +299,8 @@ final class GridDocumentHolder extends DocumentHolder { } } - if (mBullet != null && (mDetails.getVisibility() == View.GONE - || mDate.getText().isEmpty())) { + if (mBullet != null && (mDetails.getText() == null || mDetails.getText().length() == 0 + || mDate.getText() == null || mDate.getText().length() == 0)) { // There is no need for the bullet separating the details and date. mBullet.setVisibility(View.GONE); } diff --git a/src/com/android/documentsui/queries/SearchChipViewManager.java b/src/com/android/documentsui/queries/SearchChipViewManager.java index c2403dd03..bf3d1e865 100644 --- a/src/com/android/documentsui/queries/SearchChipViewManager.java +++ b/src/com/android/documentsui/queries/SearchChipViewManager.java @@ -521,7 +521,7 @@ public class SearchChipViewManager { } // Let the first checked chip can be shown. - View parent = (View) mChipGroup.getParent(); + View parent = (View) mChipGroup.getParent().getParent(); if (parent instanceof HorizontalScrollView) { final int scrollToX = isRtl ? parent.getWidth() : 0; ((HorizontalScrollView) parent).smoothScrollTo(scrollToX, 0); diff --git a/src/com/android/documentsui/util/ColorUtils.kt b/src/com/android/documentsui/util/ColorUtils.kt new file mode 100644 index 000000000..ee67b7832 --- /dev/null +++ b/src/com/android/documentsui/util/ColorUtils.kt @@ -0,0 +1,36 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.documentsui.util + +import android.content.Context +import android.util.TypedValue +import androidx.annotation.AttrRes + +class ColorUtils { + companion object { + /** + * Resolve a color attribute from the Material3 theme, example usage. + * resolveMaterialColorAttribute(context, com.google.android.material.R.attr.XXX). + */ + @JvmStatic + fun resolveMaterialColorAttribute(context: Context, @AttrRes colorAttrId: Int): Int { + val typedValue = TypedValue() + context.theme.resolveAttribute(colorAttrId, typedValue, true) + return typedValue.data + } + } +} diff --git a/tests/functional/com/android/documentsui/FilesActivityUiTest.java b/tests/functional/com/android/documentsui/FilesActivityUiTest.java index f1f505235..6c2397068 100644 --- a/tests/functional/com/android/documentsui/FilesActivityUiTest.java +++ b/tests/functional/com/android/documentsui/FilesActivityUiTest.java @@ -17,11 +17,14 @@ package com.android.documentsui; import static com.android.documentsui.flags.Flags.FLAG_HIDE_ROOTS_ON_DESKTOP_RO; +import static com.android.documentsui.flags.Flags.FLAG_USE_SEARCH_V2_READ_ONLY; +import static com.android.documentsui.flags.Flags.FLAG_USE_MATERIAL3; import android.app.Instrumentation; import android.net.Uri; import android.os.RemoteException; import android.platform.test.annotations.RequiresFlagsDisabled; +import android.platform.test.annotations.RequiresFlagsEnabled; import android.platform.test.flag.junit.CheckFlagsRule; import android.platform.test.flag.junit.DeviceFlagsValueProvider; @@ -92,13 +95,23 @@ public class FilesActivityUiTest extends ActivityTestJunit4<FilesActivity> { bots.main.assertWindowTitle("Images"); } + private void filesListed() throws Exception { + bots.directory.assertDocumentsPresent("file0.log", "file1.png", "file2.csv"); + } + @Test + @RequiresFlagsDisabled(FLAG_USE_SEARCH_V2_READ_ONLY) public void testFilesListed() throws Exception { - bots.directory.assertDocumentsPresent("file0.log", "file1.png", "file2.csv"); + filesListed(); } @Test - public void testFilesList_LiveUpdate() throws Exception { + @RequiresFlagsEnabled({FLAG_USE_SEARCH_V2_READ_ONLY, FLAG_USE_MATERIAL3}) + public void testFilesListed_searchV2() throws Exception { + filesListed(); + } + + private void filesListed_LiveUpdates() throws Exception { mDocsHelper.createDocument(rootDir0, "yummers/sandwich", "Ham & Cheese.sandwich"); bots.directory.waitForDocument("Ham & Cheese.sandwich"); @@ -107,6 +120,18 @@ public class FilesActivityUiTest extends ActivityTestJunit4<FilesActivity> { } @Test + @RequiresFlagsDisabled(FLAG_USE_SEARCH_V2_READ_ONLY) + public void testFilesList_LiveUpdate() throws Exception { + filesListed_LiveUpdates(); + } + + @Test + @RequiresFlagsEnabled({FLAG_USE_SEARCH_V2_READ_ONLY, FLAG_USE_MATERIAL3}) + public void testFilesList_LiveUpdate_searchV2() throws Exception { + filesListed_LiveUpdates(); + } + + @Test public void testNavigate_byBreadcrumb() throws Exception { bots.directory.openDocument(dirName1); bots.directory.waitForDocument(childDir1); // wait for known content diff --git a/tests/unit/com/android/documentsui/queries/SearchChipViewManagerTest.java b/tests/unit/com/android/documentsui/queries/SearchChipViewManagerTest.java index 6d20447dd..02e24a329 100644 --- a/tests/unit/com/android/documentsui/queries/SearchChipViewManagerTest.java +++ b/tests/unit/com/android/documentsui/queries/SearchChipViewManagerTest.java @@ -18,6 +18,7 @@ package com.android.documentsui.queries; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.mockito.Mockito.spy; @@ -35,6 +36,9 @@ import android.platform.test.flag.junit.DeviceFlagsValueProvider; import android.provider.DocumentsContract; import android.view.View; import android.view.ViewGroup; +import android.view.ViewParent; +import android.widget.FrameLayout; +import android.widget.HorizontalScrollView; import android.widget.LinearLayout; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -235,6 +239,37 @@ public final class SearchChipViewManagerTest { assertThat(View.VISIBLE).isEqualTo(mirror.getVisibility()); } + @Test + public void testChipChecked_resetScroll() { + // Mock chip group's parent chain according to search_chip_row.xml. + FrameLayout parent = spy(new FrameLayout(mContext)); + HorizontalScrollView grandparent = spy(new HorizontalScrollView(mContext)); + parent.addView(mChipGroup); + grandparent.addView(parent); + // Verify that getParent().getParent() returns the HorizontalScrollView mock. + ViewParent result = mChipGroup.getParent().getParent(); + assertEquals(grandparent, result); + + mSearchChipViewManager.initChipSets( + new String[] {"image/*", "audio/*", "video/*", "text/*"}); + mSearchChipViewManager.updateChips(new String[] {"*/*"}); + + // Manually set HorizontalScrollView's scrollX to something larger than 0. + grandparent.scrollTo(100, 0); + assertTrue(grandparent.getScaleX() > 0); + + assertEquals(6, mChipGroup.getChildCount()); + Chip lastChip = (Chip) mChipGroup.getChildAt(5); + + // chip.setChecked will trigger reorder animation, which needs to be run inside + // the looper thread. + InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> { + // Check last chip will move it to the first child and reset scroll view. + lastChip.setChecked(true); + assertEquals(0, grandparent.getScrollX()); + }); + } + private static Set<SearchChipData> getFakeSearchChipDataList() { final Set<SearchChipData> chipDataList = new HashSet<>(); chipDataList.add(new SearchChipData(CHIP_TYPE, 0 /* titleRes */, TEST_MIME_TYPES)); |