From af64d6af97ab2be252c32708fe2ca8c2a7d13543 Mon Sep 17 00:00:00 2001 From: Zemiao Zhu Date: Wed, 8 Apr 2020 16:02:09 -0700 Subject: Refactor SearchFragment to fix keyboard focus issue. Bug: 148623802 Test: atest DocumentsUIGoogleTests Change-Id: I4676bdffc16c5b0cbc40623e461dd397d54de610 --- res/layout/directory_header.xml | 1 + res/layout/drawer_layout.xml | 6 + res/layout/fixed_layout.xml | 18 ++- res/layout/fragment_search.xml | 15 --- src/com/android/documentsui/BaseActivity.java | 6 + .../documentsui/queries/SearchFragment.java | 134 +++++++++------------ .../documentsui/queries/SearchViewManager.java | 6 +- .../com/android/documentsui/bots/SearchBot.java | 35 ------ .../com/android/documentsui/DialogUiTest.java | 2 +- .../com/android/documentsui/SearchViewUiTest.java | 34 +----- 10 files changed, 98 insertions(+), 159 deletions(-) diff --git a/res/layout/directory_header.xml b/res/layout/directory_header.xml index 4c4d84573..94c52e978 100644 --- a/res/layout/directory_header.xml +++ b/res/layout/directory_header.xml @@ -15,6 +15,7 @@ --> + + + android:layout_weight="1"> + + + + + + - - - - - diff --git a/src/com/android/documentsui/BaseActivity.java b/src/com/android/documentsui/BaseActivity.java index 0ddf001be..5690b3deb 100644 --- a/src/com/android/documentsui/BaseActivity.java +++ b/src/com/android/documentsui/BaseActivity.java @@ -174,6 +174,10 @@ public abstract class BaseActivity */ @Override public void onSearchChanged(@Nullable String query) { + if (query != null) { + SearchFragment.dismissFragment(getSupportFragmentManager()); + } + if (mSearchManager.isSearching()) { Metrics.logSearchMode(query != null, mSearchManager.hasCheckedChip()); if (mInjector.pickResult != null) { @@ -226,6 +230,8 @@ public abstract class BaseActivity && !isInitailSearch) { SearchFragment.showFragment(getSupportFragmentManager(), mSearchManager.getSearchViewText()); + } else { + SearchFragment.dismissFragment(getSupportFragmentManager()); } } diff --git a/src/com/android/documentsui/queries/SearchFragment.java b/src/com/android/documentsui/queries/SearchFragment.java index bb7dc3e0d..525a746cf 100644 --- a/src/com/android/documentsui/queries/SearchFragment.java +++ b/src/com/android/documentsui/queries/SearchFragment.java @@ -16,26 +16,24 @@ package com.android.documentsui.queries; -import android.app.Dialog; import android.content.Context; +import android.graphics.Rect; import android.os.Bundle; -import android.text.TextUtils; import android.util.Log; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; import android.widget.AdapterView; import android.widget.ArrayAdapter; +import android.widget.FrameLayout; import android.widget.ListView; import android.widget.TextView; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import androidx.appcompat.widget.SearchView; -import androidx.appcompat.widget.Toolbar; -import androidx.fragment.app.DialogFragment; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentManager; +import androidx.fragment.app.FragmentTransaction; import com.android.documentsui.BaseActivity; import com.android.documentsui.Injector; @@ -43,8 +41,7 @@ import com.android.documentsui.R; import java.util.List; -public class SearchFragment extends DialogFragment - implements SearchView.OnQueryTextListener{ +public class SearchFragment extends Fragment{ private static final String TAG = "SearchFragment"; private static final String KEY_QUERY = "query"; @@ -52,7 +49,6 @@ public class SearchFragment extends DialogFragment private SearchViewManager mSearchViewManager; - private SearchView mSearchView; private ViewGroup mSearchChipGroup; private ListView mListView; private ArrayAdapter mAdapter; @@ -69,8 +65,17 @@ public class SearchFragment extends DialogFragment final Bundle args = new Bundle(); args.putString(KEY_QUERY, initQuery); fragment.setArguments(args); - fragment.setStyle(DialogFragment.STYLE_NO_FRAME, R.style.DocumentsTheme); - fragment.show(fm, TAG); + + final FragmentTransaction ft = fm.beginTransaction(); + ft.replace(getFragmentId(), fragment, TAG); + ft.commitNow(); + } + + public static void dismissFragment(FragmentManager fm) { + SearchFragment fragment = get(fm); + if (fragment != null) { + fragment.dismiss(); + } } public static SearchFragment get(FragmentManager fm) { @@ -80,33 +85,12 @@ public class SearchFragment extends DialogFragment : null; } - private void onChipClicked(View view) { - final Object tag = view.getTag(); - if (tag instanceof SearchChipData) { - mSearchViewManager.onMirrorChipClick((SearchChipData) tag); - dismiss(); - } - } - - private void onHistoryItemClicked(AdapterView parent, View view, int position, long id) { - final String item = mHistoryList.get(position); - mSearchViewManager.setHistorySearch(); - mSearchView.setQuery(item, true); - } - @Nullable @Override public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable Bundle savedInstanceState) { final View view = inflater.inflate(R.layout.fragment_search, container, false); - final Toolbar toolbar = view.findViewById(R.id.toolbar); - toolbar.setNavigationOnClickListener(v -> { - mSearchViewManager.cancelSearch(); - dismiss(); - }); - - mSearchView = view.findViewById(R.id.search_view); mSearchChipGroup = view.findViewById(R.id.search_chip_group); mListView = view.findViewById(R.id.history_list); @@ -123,9 +107,6 @@ public class SearchFragment extends DialogFragment final String currentQuery = getArguments().getString(KEY_QUERY, ""); - mSearchView.onActionViewExpanded(); - mSearchView.setQuery(currentQuery, false); - mSearchView.setOnQueryTextListener(this); mHistoryList = SearchHistoryManager.getInstance( getContext().getApplicationContext()).getHistoryList(currentQuery); @@ -139,56 +120,59 @@ public class SearchFragment extends DialogFragment mAdapter = new HistoryListAdapter(getContext(), mHistoryList); mListView.setAdapter(mAdapter); mListView.setOnItemClickListener(this::onHistoryItemClicked); - } - @Override - public void onStart() { - super.onStart(); - getDialog().getWindow().setLayout(ViewGroup.LayoutParams.MATCH_PARENT, - ViewGroup.LayoutParams.MATCH_PARENT); - // To avoid a11y saying button description when dialog show. - getDialog().setTitle(" "); + View toolbar = getActivity().findViewById(R.id.toolbar_background_layout); + if (toolbar != null) { + // Align top with the bottom of search bar. + Rect rect = new Rect(); + toolbar.getGlobalVisibleRect(rect); + FrameLayout.LayoutParams layoutParams = new FrameLayout.LayoutParams( + ViewGroup.LayoutParams.MATCH_PARENT, + ViewGroup.LayoutParams.MATCH_PARENT); + layoutParams.setMargins(0, rect.height(), 0, 0); + getView().setLayoutParams(layoutParams); + } + + updateDirectoryVisibility(View.GONE); } - @Override - public Dialog onCreateDialog(@Nullable Bundle savedInstanceState) { - return new Dialog(getActivity(), getTheme()){ - @Override - public void onBackPressed() { - if (TextUtils.isEmpty(mSearchView.getQuery())) { - mSearchViewManager.cancelSearch(); - } else { - mSearchViewManager.restoreSearch(false); - } - dismiss(); - } - }; + private static int getFragmentId() { + return R.id.container_search_fragment; } - @Override - public boolean onQueryTextSubmit(String s) { - if (!TextUtils.isEmpty(mSearchView.getQuery())) { - mSearchViewManager.setCurrentSearch(s); - mSearchViewManager.restoreSearch(false); - mSearchViewManager.recordHistory(); - dismiss(); + private void onChipClicked(View view) { + final Object tag = view.getTag(); + if (tag instanceof SearchChipData) { + mSearchViewManager.onMirrorChipClick((SearchChipData) tag); } - return true; } - @Override - public boolean onQueryTextChange(String s) { - if (!TextUtils.isEmpty(mSearchView.getQuery())) { - mSearchViewManager.setCurrentSearch(s); - mSearchViewManager.restoreSearch(true); - dismiss(); - } else { - mHistoryList = SearchHistoryManager.getInstance( - mSearchView.getContext().getApplicationContext()).getHistoryList(""); - mAdapter.clear(); - mAdapter.addAll(mHistoryList); + private void onHistoryItemClicked(AdapterView parent, View view, int position, long id) { + final String item = mHistoryList.get(position); + mSearchViewManager.setHistorySearch(); + mSearchViewManager.setCurrentSearch(item); + mSearchViewManager.restoreSearch(true); + dismiss(); + } + + private void dismiss() { + updateDirectoryVisibility(View.VISIBLE); + + FragmentTransaction ft = getParentFragmentManager().beginTransaction(); + ft.remove(this); + ft.commitNow(); + } + + private void updateDirectoryVisibility(int visibility) { + View directoryHeader = getActivity().findViewById(R.id.directory_header); + if (directoryHeader != null) { + directoryHeader.setVisibility(visibility); + } + + View directoryContainer = getActivity().findViewById(R.id.container_directory); + if (directoryContainer != null) { + directoryContainer.setVisibility(visibility); } - return true; } private class HistoryListAdapter extends ArrayAdapter { diff --git a/src/com/android/documentsui/queries/SearchViewManager.java b/src/com/android/documentsui/queries/SearchViewManager.java index c4b91d54f..e0528328c 100644 --- a/src/com/android/documentsui/queries/SearchViewManager.java +++ b/src/com/android/documentsui/queries/SearchViewManager.java @@ -119,7 +119,8 @@ public class SearchViewManager implements mChipViewManager.setSearchChipViewManagerListener(this::onChipCheckedStateChanged); if (savedState != null) { - mCurrentSearch = savedState.getString(Shared.EXTRA_QUERY); + String savedQuery = savedState.getString(Shared.EXTRA_QUERY); + mCurrentSearch = savedQuery != null ? savedQuery : ""; mChipViewManager.restoreCheckedChipItems(savedState); } else { mCurrentSearch = null; @@ -223,6 +224,7 @@ public class SearchViewManager implements if (clearButton != null) { clearButton.setOnClickListener(v -> { mSearchView.setQuery("", false); + mSearchView.requestFocus(); mListener.onSearchViewClearClicked(); }); } @@ -232,7 +234,7 @@ public class SearchViewManager implements mSearchView.setMaxWidth(Integer.MAX_VALUE); mMenuItem.setOnActionExpandListener(this); - restoreSearch(false); + restoreSearch(true); } /** diff --git a/tests/common/com/android/documentsui/bots/SearchBot.java b/tests/common/com/android/documentsui/bots/SearchBot.java index 9c3568bc5..da7dc5481 100644 --- a/tests/common/com/android/documentsui/bots/SearchBot.java +++ b/tests/common/com/android/documentsui/bots/SearchBot.java @@ -69,9 +69,6 @@ public class SearchBot extends Bots.BaseBot { public void clickIcon() throws UiObjectNotFoundException { UiObject searchView = findSearchView(); searchView.click(); - - UiObject fragmentSearchView = findFragmentSearchView(); - assertTrue(fragmentSearchView.exists()); } public void clickSearchViewClearButton() throws UiObjectNotFoundException { @@ -79,11 +76,6 @@ public class SearchBot extends Bots.BaseBot { clear.click(); } - public void clickFragmentSearchViewClearButton() throws UiObjectNotFoundException { - UiObject clear = findFragmentSearchClearButton(); - clear.click(); - } - public void setInputText(String query) throws UiObjectNotFoundException { onView(SEARCH_INPUT).perform(typeText(query)); } @@ -121,19 +113,6 @@ public class SearchBot extends Bots.BaseBot { assertEquals(exists, findSearchViewTextField().exists()); } - public void assertFragmentInputFocused(boolean focused) - throws UiObjectNotFoundException { - UiObject textField = findFragmentSearchViewTextField(); - - assertTrue(textField.exists()); - assertEquals(focused, textField.isFocused()); - } - - public void assertFragmentInputExists(boolean exists) - throws UiObjectNotFoundException { - assertEquals(exists, findFragmentSearchViewTextField().exists()); - } - private UiObject findSearchView() { return findObject(mTargetPackage + ":id/option_menu_search"); } @@ -148,20 +127,6 @@ public class SearchBot extends Bots.BaseBot { mTargetPackage + ":id/search_close_btn"); } - private UiObject findFragmentSearchView() { - return findObject(mTargetPackage + ":id/search_view"); - } - - private UiObject findFragmentSearchViewTextField() { - return findObject(mTargetPackage + ":id/search_view", - mTargetPackage + ":id/search_src_text"); - } - - private UiObject findFragmentSearchClearButton() { - return findObject(mTargetPackage + ":id/search_view", - mTargetPackage + ":id/search_close_btn"); - } - private UiObject findSearchViewIcon() { return mContext.getResources().getBoolean(R.bool.full_bar_search_view) ? findObject(mTargetPackage + ":id/option_menu_search") diff --git a/tests/functional/com/android/documentsui/DialogUiTest.java b/tests/functional/com/android/documentsui/DialogUiTest.java index 24136021d..42234afd3 100644 --- a/tests/functional/com/android/documentsui/DialogUiTest.java +++ b/tests/functional/com/android/documentsui/DialogUiTest.java @@ -104,7 +104,7 @@ public class DialogUiTest { assertNotNull("Dialog was null", mCreateDirectoryFragment.getDialog()); assertTrue("Dialog was not being shown", mCreateDirectoryFragment.getDialog().isShowing()); - mActivityTestRule.runOnUiThread(() -> mCreateDirectoryFragment.dismiss()); + mActivityTestRule.runOnUiThread(() -> mCreateDirectoryFragment.getDialog().dismiss()); InstrumentationRegistry.getInstrumentation().waitForIdleSync(); assertNull("Dialog should be null after dismiss()", mCreateDirectoryFragment.getDialog()); diff --git a/tests/functional/com/android/documentsui/SearchViewUiTest.java b/tests/functional/com/android/documentsui/SearchViewUiTest.java index d18ac3c02..bc08c4984 100644 --- a/tests/functional/com/android/documentsui/SearchViewUiTest.java +++ b/tests/functional/com/android/documentsui/SearchViewUiTest.java @@ -57,7 +57,6 @@ public class SearchViewUiTest extends ActivityTest { public void testSearchIconVisible() throws Exception { // The default root (root 0) supports search bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); bots.search.assertIconVisible(true); } @@ -67,15 +66,14 @@ public class SearchViewUiTest extends ActivityTest { bots.search.assertIconVisible(false); bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); } public void testSearchView_ExpandsOnClick() throws Exception { bots.search.clickIcon(); device.waitForIdle(); - bots.search.assertFragmentInputExists(true); - bots.search.assertFragmentInputFocused(true); + bots.search.assertInputExists(true); + bots.search.assertInputFocused(true); // FIXME: Matchers fail the not-present check if we've ever clicked this. // bots.search.assertIconVisible(false); @@ -85,8 +83,8 @@ public class SearchViewUiTest extends ActivityTest { bots.search.clickIcon(); device.waitForIdle(); - bots.search.assertFragmentInputExists(true); - bots.search.assertFragmentInputFocused(true); + bots.search.assertInputExists(true); + bots.search.assertInputFocused(true); device.waitForIdle(); assertFalse(bots.menu.hasMenuItem("Grid view")); @@ -97,11 +95,9 @@ public class SearchViewUiTest extends ActivityTest { public void testSearchView_CollapsesOnBack() throws Exception { bots.search.clickIcon(); device.pressBack(); - device.pressBack(); bots.search.assertIconVisible(true); bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); } public void testSearchView_ClearsTextOnBack() throws Exception { @@ -116,7 +112,6 @@ public class SearchViewUiTest extends ActivityTest { bots.search.assertIconVisible(true); bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); } public void testSearchView_ClearsSearchOnBack() throws Exception { @@ -129,7 +124,6 @@ public class SearchViewUiTest extends ActivityTest { bots.search.assertIconVisible(true); bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); } public void testSearchView_ClearsAutoSearchOnBack() throws Exception { @@ -143,7 +137,6 @@ public class SearchViewUiTest extends ActivityTest { bots.search.assertIconVisible(true); bots.search.assertInputExists(false); - bots.search.assertFragmentInputExists(false); } public void testSearchView_StateAfterSearch() throws Exception { @@ -232,22 +225,7 @@ public class SearchViewUiTest extends ActivityTest { bots.search.clickSearchViewClearButton(); device.waitForIdle(); - bots.search.assertFragmentInputExists(true); - bots.search.assertFragmentInputFocused(true); - } - - public void testSearchHistory_showAfterFragmentSearchViewClear() throws Exception { - bots.search.clickIcon(); - bots.search.setInputText("chocolate"); - - bots.keyboard.pressEnter(); - device.waitForIdle(); - - bots.search.clickIcon(); - bots.search.clickFragmentSearchViewClearButton(); - device.waitForIdle(); - - bots.search.assertFragmentInputExists(true); - bots.search.assertFragmentInputFocused(true); + bots.search.assertInputExists(true); + bots.search.assertInputFocused(true); } } -- cgit v1.2.3-59-g8ed1b