From f6a31d319f3e08d26ca4c0fbfb5803056bafb243 Mon Sep 17 00:00:00 2001 From: Aga Wronska Date: Fri, 15 Jan 2016 17:30:15 -0800 Subject: [PATCH] Refactor SearchManager: - Move SearchManager out of BaseActivity. - Refactor usage of onCurrentDirectoryChanged(int anim). - Cancel search when directory changes. - Enable testing for cancelling search when directory changes. - Avoid recreating action bar when only the content of the directory is changed. BUG=26495573 Change-Id: I1400090fcb529ad3905018d1640fdb7416542989 --- .../com/android/documentsui/BaseActivity.java | 227 +++--------------- .../documentsui/DocumentsActivity.java | 8 +- .../documentsui/DownloadsActivity.java | 6 +- .../android/documentsui/FilesActivity.java | 8 +- .../android/documentsui/SearchManager.java | 210 ++++++++++++++++ .../dirlist/DirectoryFragment.java | 2 +- .../android/documentsui/SearchViewUiTest.java | 13 +- 7 files changed, 267 insertions(+), 207 deletions(-) create mode 100644 packages/DocumentsUI/src/com/android/documentsui/SearchManager.java diff --git a/packages/DocumentsUI/src/com/android/documentsui/BaseActivity.java b/packages/DocumentsUI/src/com/android/documentsui/BaseActivity.java index be544967355f2..0fb8dafb91901 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/BaseActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/BaseActivity.java @@ -38,24 +38,21 @@ import android.provider.DocumentsContract; import android.provider.DocumentsContract.Root; import android.support.annotation.LayoutRes; import android.support.annotation.Nullable; -import android.text.TextUtils; import android.util.Log; import android.view.LayoutInflater; import android.view.Menu; import android.view.MenuItem; import android.view.View; -import android.view.View.OnClickListener; -import android.view.View.OnFocusChangeListener; import android.view.ViewGroup; import android.widget.AdapterView; import android.widget.AdapterView.OnItemSelectedListener; import android.widget.BaseAdapter; import android.widget.ImageView; -import android.widget.SearchView; -import android.widget.SearchView.OnQueryTextListener; import android.widget.TextView; import com.android.documentsui.RecentsProvider.ResumeColumns; +import com.android.documentsui.SearchManager; +import com.android.documentsui.SearchManager.SearchManagerListener; import com.android.documentsui.dirlist.DirectoryFragment; import com.android.documentsui.model.DocumentInfo; import com.android.documentsui.model.DocumentStack; @@ -72,7 +69,7 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.Executor; -public abstract class BaseActivity extends Activity { +public abstract class BaseActivity extends Activity implements SearchManagerListener { static final String EXTRA_STATE = "state"; @@ -91,7 +88,7 @@ public abstract class BaseActivity extends Activity { public abstract void onDocumentsPicked(List docs); abstract void onTaskFinished(Uri... uris); - abstract void onDirectoryChanged(int anim); + abstract void refreshDirectory(int anim); abstract void updateActionBar(); abstract void saveStackBlocking(); abstract State buildState(); @@ -121,7 +118,7 @@ public abstract class BaseActivity extends Activity { } }); mDirectoryContainer = (DirectoryContainerView) findViewById(R.id.container_directory); - mSearchManager = new SearchManager(); + mSearchManager = new SearchManager(this); // Base classes must update result in their onCreate. setResult(Activity.RESULT_CANCELED); @@ -213,7 +210,7 @@ public abstract class BaseActivity extends Activity { // Otherwise we delegate loading data from disk to a task // to ensure a responsive ui. if (mRoots.isRecentsRoot(root)) { - onCurrentDirectoryChanged(ANIM_SIDE); + refreshCurrentRootAndDirectory(ANIM_SIDE); } else { new PickRootTask(root, true).executeOnExecutor(getExecutorForCurrentDirectory()); } @@ -319,19 +316,19 @@ public abstract class BaseActivity extends Activity { void openContainerDocument(DocumentInfo doc) { checkArgument(doc.isContainer()); mState.pushDocument(doc); - onCurrentDirectoryChanged(ANIM_DOWN); + refreshCurrentRootAndDirectory(ANIM_DOWN); } /** - * Call this when directory changes. Prior to root fragment update - * the (abstract) directoryChanged method will be called. + * Refreshes the content of the director and the menu/action bar. + * The current directory name and selection will get updated. * @param anim */ - // TODO: Refactor the usage of the method - now it is called not only when the directory - // changed, but also to refresh the content of the directory while searching - final void onCurrentDirectoryChanged(int anim) { + final void refreshCurrentRootAndDirectory(int anim) { + mSearchManager.cancelSearch(); + mDirectoryContainer.setDrawDisappearingFirst(anim == ANIM_DOWN); - onDirectoryChanged(anim); + refreshDirectory(anim); final RootsFragment roots = RootsFragment.get(getFragmentManager()); if (roots != null) { @@ -340,10 +337,28 @@ public abstract class BaseActivity extends Activity { updateActionBar(); - // Prevents searchView from being recreated while searching - if (!mSearchManager.isSearching()) { - invalidateOptionsMenu(); - } + invalidateOptionsMenu(); + } + + /** + * Called when search results changed. + * Refreshes the content of the directory. It doesn't refresh elements on the action bar. + * e.g. The current directory name displayed on the action bar won't get updated. + */ + @Override + public void onSearchChanged() { + mDirectoryContainer.setDrawDisappearingFirst(false); + refreshDirectory(ANIM_NONE); + } + + /** + * Called when search query changed. + * Updates the state object. + * @param query - New query + */ + @Override + public void onSearchQueryChanged(String query) { + mState.currentSearch = query; } final List getExcludedAuthorities() { @@ -479,7 +494,7 @@ public abstract class BaseActivity extends Activity { mDrawer.setOpen(false); } else if (size > 1) { mState.stack.pop(); - onCurrentDirectoryChanged(ANIM_UP); + refreshCurrentRootAndDirectory(ANIM_UP); } else { super.onBackPressed(); } @@ -490,7 +505,7 @@ public abstract class BaseActivity extends Activity { // Update the restored stack to ensure we have freshest data stack.updateDocuments(getContentResolver()); mState.setStack(stack); - onCurrentDirectoryChanged(ANIM_SIDE); + refreshCurrentRootAndDirectory(ANIM_SIDE); } catch (FileNotFoundException e) { Log.w(mTag, "Failed to restore stack: " + e); @@ -579,7 +594,7 @@ public abstract class BaseActivity extends Activity { protected void onPostExecute(Void result) { if (isDestroyed()) return; mState.restored = true; - onCurrentDirectoryChanged(ANIM_NONE); + refreshCurrentRootAndDirectory(ANIM_NONE); onStackRestored(mRestoredStack, mExternal); } } @@ -659,7 +674,7 @@ public abstract class BaseActivity extends Activity { while (mState.stack.size() > position + 1) { mState.popDocument(); } - onCurrentDirectoryChanged(ANIM_UP); + refreshCurrentRootAndDirectory(ANIM_UP); } @Override @@ -731,170 +746,6 @@ public abstract class BaseActivity extends Activity { } } - /** - * Facade over the various search parts in the menu. - */ - final class SearchManager implements - SearchView.OnCloseListener, OnQueryTextListener, OnClickListener, OnFocusChangeListener, - DocumentsToolBar.OnActionViewCollapsedListener { - - private boolean mSearchExpanded; - private boolean mIgnoreNextClose; - private boolean mIgnoreNextCollapse; - - private DocumentsToolBar mActionBar; - private MenuItem mMenu; - private SearchView mView; - - public void install(DocumentsToolBar actionBar) { - assert(mActionBar == null); - mActionBar = actionBar; - mMenu = actionBar.getSearchMenu(); - mView = (SearchView) mMenu.getActionView(); - - mActionBar.setOnActionViewCollapsedListener(this); - mView.setOnQueryTextListener(this); - mView.setOnCloseListener(this); - mView.setOnSearchClickListener(this); - mView.setOnQueryTextFocusChangeListener(this); - } - - /** - * @param root Info about the current directory. - */ - void update(RootInfo root) { - if (mMenu == null) { - Log.d(mTag, "update called before Search MenuItem installed."); - return; - } - - if (mState.currentSearch != null) { - mMenu.expandActionView(); - - mView.setIconified(false); - mView.clearFocus(); - mView.setQuery(mState.currentSearch, false); - } else { - mView.clearFocus(); - if (!mView.isIconified()) { - mIgnoreNextClose = true; - mView.setIconified(true); - } - - if (mMenu.isActionViewExpanded()) { - mIgnoreNextCollapse = true; - mMenu.collapseActionView(); - } - } - - showMenu(root != null - && ((root.flags & Root.FLAG_SUPPORTS_SEARCH) != 0)); - } - - void showMenu(boolean visible) { - if (mMenu == null) { - Log.d(mTag, "showMenu called before Search MenuItem installed."); - return; - } - - mMenu.setVisible(visible); - if (!visible) { - mState.currentSearch = null; - } - } - - /** - * Cancels current search operation. - * @return True if it cancels search. False if it does not operate - * search currently. - */ - boolean cancelSearch() { - if (isExpanded() || isSearching()) { - // If the query string is not empty search view won't get iconified - mView.setQuery("", false); - mView.setIconified(true); - return true; - } - return false; - } - - boolean isSearching() { - return mState.currentSearch != null; - } - - boolean isExpanded() { - return mSearchExpanded; - } - - /** - * Clears the search. - * @return True if the default behavior of clearing/dismissing SearchView should be - * overridden. False otherwise. - */ - @Override - public boolean onClose() { - mSearchExpanded = false; - if (mIgnoreNextClose) { - mIgnoreNextClose = false; - return false; - } - - mView.setBackgroundColor( - getResources().getColor(android.R.color.transparent, null)); - - // Refresh the directory if a search was done - if(mState.currentSearch != null) { - mState.currentSearch = null; - onCurrentDirectoryChanged(ANIM_NONE); - } - - return false; - } - - /** - * Sets mSearchExpanded. - * Called when search icon is clicked to start search. - * Used to detect when the view expanded instead of onMenuItemActionExpand, because - * SearchView has showAsAction set to always and onMenuItemAction* methods are not called. - */ - @Override - public void onClick (View v) { - mSearchExpanded = true; - mView.setBackgroundColor( - getResources().getColor(R.color.menu_search_background, null)); - } - - @Override - public boolean onQueryTextSubmit(String query) { - mState.currentSearch = query; - mView.clearFocus(); - onCurrentDirectoryChanged(ANIM_NONE); - return true; - } - - @Override - public boolean onQueryTextChange(String newText) { - return false; - } - - @Override - public void onFocusChange(View v, boolean hasFocus) { - if(!hasFocus) { - if(mState.currentSearch == null) { - mView.setIconified(true); - } - else if(TextUtils.isEmpty(mView.getQuery())) { - cancelSearch(); - } - } - } - - @Override - public void onActionViewCollapsed() { - updateActionBar(); - } - } - /** * Interface providing access to current view of documents * even when all documents are not homed to the same parent. diff --git a/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java b/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java index 223af896ae230..6dcd472b3efe2 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/DocumentsActivity.java @@ -120,7 +120,7 @@ public class DocumentsActivity extends BaseActivity { setTitle(""); new RestoreStackTask().execute(); } else { - onCurrentDirectoryChanged(ANIM_NONE); + refreshCurrentRootAndDirectory(ANIM_NONE); } } @@ -332,7 +332,7 @@ public class DocumentsActivity extends BaseActivity { } @Override - void onDirectoryChanged(int anim) { + void refreshDirectory(int anim) { final FragmentManager fm = getFragmentManager(); final RootInfo root = getCurrentRoot(); final DocumentInfo cwd = getCurrentDirectory(); @@ -353,12 +353,12 @@ public class DocumentsActivity extends BaseActivity { mState.derivedMode = mState.userMode; } } else { - if (mState.currentSearch != null) { + if (mSearchManager.isSearching()) { // Ongoing search DirectoryFragment.showSearch(fm, root, mState.currentSearch, anim); } else { // Normal boring directory - DirectoryFragment.showNormal(fm, root, cwd, anim); + DirectoryFragment.showDirectory(fm, root, cwd, anim); } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/DownloadsActivity.java b/packages/DocumentsUI/src/com/android/documentsui/DownloadsActivity.java index b806ced42c6bf..3302da97bcae9 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/DownloadsActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/DownloadsActivity.java @@ -92,7 +92,7 @@ public class DownloadsActivity extends BaseActivity { final Uri rootUri = getIntent().getData(); new RestoreRootTask(rootUri).executeOnExecutor(getExecutorForCurrentDirectory()); } else { - onCurrentDirectoryChanged(ANIM_NONE); + refreshCurrentRootAndDirectory(ANIM_NONE); } } @@ -164,7 +164,7 @@ public class DownloadsActivity extends BaseActivity { } @Override - void onDirectoryChanged(int anim) { + void refreshDirectory(int anim) { final FragmentManager fm = getFragmentManager(); final RootInfo root = getCurrentRoot(); final DocumentInfo cwd = getCurrentDirectory(); @@ -178,7 +178,7 @@ public class DownloadsActivity extends BaseActivity { DirectoryFragment.showSearch(fm, root, mState.currentSearch, anim); } else { // Normal boring directory - DirectoryFragment.showNormal(fm, root, cwd, anim); + DirectoryFragment.showDirectory(fm, root, cwd, anim); } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/FilesActivity.java b/packages/DocumentsUI/src/com/android/documentsui/FilesActivity.java index 0bd09f68ee198..58537ee495a8d 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/FilesActivity.java +++ b/packages/DocumentsUI/src/com/android/documentsui/FilesActivity.java @@ -95,7 +95,7 @@ public class FilesActivity extends BaseActivity { if (mState.restored) { if (DEBUG) Log.d(TAG, "Stack already resolved for uri: " + intent.getData()); - onCurrentDirectoryChanged(ANIM_NONE); + refreshCurrentRootAndDirectory(ANIM_NONE); } else if (!mState.stack.isEmpty()) { // If a non-empty stack is present in our state it was read (presumably) // from EXTRA_STACK intent extra. In this case, we'll skip other means of @@ -106,7 +106,7 @@ public class FilesActivity extends BaseActivity { // don't specify a real content target. if (DEBUG) Log.d(TAG, "Launching with non-empty stack."); checkState(uri == null || LauncherActivity.isLaunchUri(uri)); - onCurrentDirectoryChanged(ANIM_NONE); + refreshCurrentRootAndDirectory(ANIM_NONE); } else if (DocumentsContract.isRootUri(this, uri)) { if (DEBUG) Log.d(TAG, "Launching with root URI."); // If we've got a specific root to display, restore that root using a dedicated @@ -288,7 +288,7 @@ public class FilesActivity extends BaseActivity { } @Override - void onDirectoryChanged(int anim) { + void refreshDirectory(int anim) { final FragmentManager fm = getFragmentManager(); final RootInfo root = getCurrentRoot(); final DocumentInfo cwd = getCurrentDirectory(); @@ -307,7 +307,7 @@ public class FilesActivity extends BaseActivity { DirectoryFragment.showSearch(fm, root, mState.currentSearch, anim); } else { // Normal boring directory - DirectoryFragment.showNormal(fm, root, cwd, anim); + DirectoryFragment.showDirectory(fm, root, cwd, anim); } } } diff --git a/packages/DocumentsUI/src/com/android/documentsui/SearchManager.java b/packages/DocumentsUI/src/com/android/documentsui/SearchManager.java new file mode 100644 index 0000000000000..fb585a618c476 --- /dev/null +++ b/packages/DocumentsUI/src/com/android/documentsui/SearchManager.java @@ -0,0 +1,210 @@ +/* + * Copyright (C) 2013 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; + +import android.provider.DocumentsContract.Root; +import android.text.TextUtils; +import android.util.Log; +import android.view.MenuItem; +import android.view.View; +import android.view.View.OnClickListener; +import android.view.View.OnFocusChangeListener; +import android.widget.SearchView; +import android.widget.SearchView.OnQueryTextListener; + +import com.android.documentsui.model.RootInfo; + +/** + * Manages searching UI behavior. + */ +final class SearchManager implements + SearchView.OnCloseListener, OnQueryTextListener, OnClickListener, OnFocusChangeListener { + + public interface SearchManagerListener { + void onSearchChanged(); + + void onSearchQueryChanged(String query); + } + + public static final String TAG = "SearchManger"; + + private SearchManagerListener mListener; + private String currentSearch; + private boolean mSearchExpanded; + private boolean mIgnoreNextClose; + + private DocumentsToolBar mActionBar; + private MenuItem mMenu; + private SearchView mView; + + public SearchManager(SearchManagerListener listener) { + mListener = listener; + } + + public void setSearchMangerListener(SearchManagerListener listener) { + mListener = listener; + } + + public void install(DocumentsToolBar actionBar) { + assert (mActionBar == null); + mActionBar = actionBar; + mMenu = actionBar.getSearchMenu(); + mView = (SearchView) mMenu.getActionView(); + + mView.setOnQueryTextListener(this); + mView.setOnCloseListener(this); + mView.setOnSearchClickListener(this); + mView.setOnQueryTextFocusChangeListener(this); + } + + /** + * @param root Info about the current directory. + */ + void update(RootInfo root) { + if (mMenu == null) { + Log.d(TAG, "update called before Search MenuItem installed."); + return; + } + + if (currentSearch != null) { + mMenu.expandActionView(); + + mView.setIconified(false); + mView.clearFocus(); + mView.setQuery(currentSearch, false); + } else { + mView.clearFocus(); + if (!mView.isIconified()) { + mIgnoreNextClose = true; + mView.setIconified(true); + } + + if (mMenu.isActionViewExpanded()) { + mMenu.collapseActionView(); + } + } + + showMenu(root != null + && ((root.flags & Root.FLAG_SUPPORTS_SEARCH) != 0)); + } + + void showMenu(boolean visible) { + if (mMenu == null) { + Log.d(TAG, "showMenu called before Search MenuItem installed."); + return; + } + + mMenu.setVisible(visible); + if (!visible) { + currentSearch = null; + if (mListener != null) { + mListener.onSearchQueryChanged(currentSearch); + } + } + } + + /** + * Cancels current search operation. Triggers clearing and collapsing the SearchView. + * + * @return True if it cancels search. False if it does not operate search currently. + */ + boolean cancelSearch() { + if (isExpanded() || isSearching()) { + // If the query string is not empty search view won't get iconified + mView.setQuery("", false); + // Causes calling onClose(). onClose() is triggering directory content update. + mView.setIconified(true); + return true; + } + return false; + } + + boolean isSearching() { + return currentSearch != null; + } + + boolean isExpanded() { + return mSearchExpanded; + } + + /** + * Clears the search. Clears the SearchView background color. Triggers refreshing of the + * directory content. + * @return True if the default behavior of clearing/dismissing SearchView should be overridden. + * False otherwise. + */ + @Override + public boolean onClose() { + mSearchExpanded = false; + if (mIgnoreNextClose) { + mIgnoreNextClose = false; + return false; + } + + mView.setBackgroundColor( + mView.getResources().getColor(android.R.color.transparent, null)); + + // Refresh the directory if a search was done + if (currentSearch != null) { + currentSearch = null; + if (mListener != null) { + mListener.onSearchQueryChanged(currentSearch); + mListener.onSearchChanged(); + } + } + return false; + } + + /** + * Sets mSearchExpanded. Called when search icon is clicked to start search. Used to detect when + * the view expanded instead of onMenuItemActionExpand, because SearchView has showAsAction set + * to always and onMenuItemAction* methods are not called. + */ + @Override + public void onClick(View v) { + mSearchExpanded = true; + mView.setBackgroundColor( + mView.getResources().getColor(R.color.menu_search_background, null)); + } + + @Override + public boolean onQueryTextSubmit(String query) { + currentSearch = query; + mView.clearFocus(); + if (mListener != null) { + mListener.onSearchQueryChanged(currentSearch); + mListener.onSearchChanged(); + } + return true; + } + + @Override + public void onFocusChange(View v, boolean hasFocus) { + if (!hasFocus) { + if (currentSearch == null) { + mView.setIconified(true); + } else if (TextUtils.isEmpty(mView.getQuery())) { + cancelSearch(); + } + } + } + + @Override + public boolean onQueryTextChange(String newText) { + return false; + } +} diff --git a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java index 22e81c6c07ad6..580e2d8d97b74 100644 --- a/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java +++ b/packages/DocumentsUI/src/com/android/documentsui/dirlist/DirectoryFragment.java @@ -163,7 +163,7 @@ public class DirectoryFragment extends Fragment implements DocumentsAdapter.Envi private MessageBar mMessageBar; private View mProgressBar; - public static void showNormal(FragmentManager fm, RootInfo root, DocumentInfo doc, int anim) { + public static void showDirectory(FragmentManager fm, RootInfo root, DocumentInfo doc, int anim) { show(fm, TYPE_NORMAL, root, doc, null, anim); } diff --git a/packages/DocumentsUI/tests/src/com/android/documentsui/SearchViewUiTest.java b/packages/DocumentsUI/tests/src/com/android/documentsui/SearchViewUiTest.java index baa7a2ee17863..1b8bd4e5b5b23 100644 --- a/packages/DocumentsUI/tests/src/com/android/documentsui/SearchViewUiTest.java +++ b/packages/DocumentsUI/tests/src/com/android/documentsui/SearchViewUiTest.java @@ -259,12 +259,11 @@ public class SearchViewUiTest extends InstrumentationTestCase { mBot.openRoot(ROOT_1_ID); - // This assert is failing right now - fix will come with SearchManager refactoring - // assertDefaultTestDir1(); - // - // mBot.openRoot(ROOT_0_ID); - // - // assertDefaultTestDir0(); + assertDefaultTestDir1(); + + mBot.openRoot(ROOT_0_ID); + + assertDefaultTestDir0(); } private void assertDefaultTestDir0() throws UiObjectNotFoundException { @@ -280,7 +279,7 @@ public class SearchViewUiTest extends InstrumentationTestCase { assertFalse(mSearchTextField.exists()); assertTrue(mDocsList.exists()); assertEquals(2, mDocsList.getChildCount()); - mBot.assertHasDocuments("anotherFile0.log", "poodles.txt"); + mBot.assertHasDocuments("anotherFile0.log", "poodles.text"); } private void assertSearchTextField(boolean isFocused, String query)