Commit eb78c9a8 authored by Friedrich Horschig's avatar Friedrich Horschig Committed by Commit Bot

Fix the password search icon and view layout

The "ifRoom" property doesn't work properly with action views as the
main item will not collapse in that case and just move the option to
the settings menu.
Using "always" (like Site Settings does it) fixes that issue.
Removing the previously used "collapseActionView" prevents filling
the entire action bar which fixes layouting on large screens.

All other changes ensure no regressions although we now have to rely
on different Listener methods and UI interaction flows.


Bug: 809014, 807303
Change-Id: I63a07b8b91077e33c50e4859889474d072cb7c94
Reviewed-on: https://chromium-review.googlesource.com/909217
Commit-Queue: Friedrich Horschig <fhorschig@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#536065}
parent abf5cdce
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
android:id="@+id/menu_id_search" android:id="@+id/menu_id_search"
android:icon="@drawable/ic_search" android:icon="@drawable/ic_search"
android:title="@string/search" android:title="@string/search"
chrome:showAsAction="ifRoom|collapseActionView" chrome:showAsAction="always"
chrome:actionViewClass="android.support.v7.widget.SearchView" /> chrome:actionViewClass="android.support.v7.widget.SearchView" />
<item <item
......
...@@ -28,6 +28,7 @@ import android.text.style.ForegroundColorSpan; ...@@ -28,6 +28,7 @@ import android.text.style.ForegroundColorSpan;
import android.view.Menu; import android.view.Menu;
import android.view.MenuInflater; import android.view.MenuInflater;
import android.view.MenuItem; import android.view.MenuItem;
import android.view.View;
import android.view.inputmethod.EditorInfo; import android.view.inputmethod.EditorInfo;
import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.ApiCompatibilityUtils;
...@@ -148,6 +149,7 @@ public class SavePasswordsPreferences ...@@ -148,6 +149,7 @@ public class SavePasswordsPreferences
@Nullable @Nullable
private Uri mExportFileUri; private Uri mExportFileUri;
private MenuItem mHelpItem;
private String mSearchQuery; private String mSearchQuery;
private Preference mLinkPref; private Preference mLinkPref;
private ChromeSwitchPreference mSavePasswordsSwitch; private ChromeSwitchPreference mSavePasswordsSwitch;
...@@ -212,6 +214,7 @@ public class SavePasswordsPreferences ...@@ -212,6 +214,7 @@ public class SavePasswordsPreferences
MenuItem searchItem = menu.findItem(R.id.menu_id_search); MenuItem searchItem = menu.findItem(R.id.menu_id_search);
searchItem.setVisible(providesPasswordSearch()); searchItem.setVisible(providesPasswordSearch());
if (providesPasswordSearch()) { if (providesPasswordSearch()) {
mHelpItem = menu.findItem(R.id.menu_id_general_help);
setUpSearchAction(searchItem); setUpSearchAction(searchItem);
} }
} }
...@@ -229,20 +232,14 @@ public class SavePasswordsPreferences ...@@ -229,20 +232,14 @@ public class SavePasswordsPreferences
searchItem.expandActionView(); searchItem.expandActionView();
searchView.setQuery(mSearchQuery, false); searchView.setQuery(mSearchQuery, false);
} }
searchItem.setOnActionExpandListener(new MenuItem.OnActionExpandListener() { searchItem.setOnMenuItemClickListener((MenuItem m) -> {
@Override filterPasswords("");
public boolean onMenuItemActionExpand(MenuItem menuItem) { return false; // Continue with the default action.
maybeRecordTriggeredPasswordSearch(true); });
filterPasswords(""); // Hide other menu elements. searchView.findViewById(R.id.search_close_btn).setOnClickListener((View v) -> {
return true; // Continue expanding. searchView.setQuery(null, false);
} searchView.setIconified(true);
filterPasswords(null); // Reset filter to bring back all preferences.
@Override
public boolean onMenuItemActionCollapse(MenuItem menuItem) {
searchView.setQuery(null, false);
filterPasswords(null); // Reset filter to bring back all preferences.
return true; // Continue collapsing.
}
}); });
searchView.setOnSearchClickListener(view -> filterPasswords("")); searchView.setOnSearchClickListener(view -> filterPasswords(""));
searchView.setOnQueryTextListener(new SearchView.OnQueryTextListener() { searchView.setOnQueryTextListener(new SearchView.OnQueryTextListener() {
...@@ -253,6 +250,7 @@ public class SavePasswordsPreferences ...@@ -253,6 +250,7 @@ public class SavePasswordsPreferences
@Override @Override
public boolean onQueryTextChange(String query) { public boolean onQueryTextChange(String query) {
maybeRecordTriggeredPasswordSearch(true);
return filterPasswords(query); return filterPasswords(query);
} }
}); });
...@@ -573,6 +571,9 @@ public class SavePasswordsPreferences ...@@ -573,6 +571,9 @@ public class SavePasswordsPreferences
private boolean filterPasswords(String query) { private boolean filterPasswords(String query) {
mSearchQuery = query; mSearchQuery = query;
// Hide the help option. It's not useful during search but might be clicked by accident.
mHelpItem.setShowAsAction(mSearchQuery == null ? MenuItem.SHOW_AS_ACTION_NEVER
: MenuItem.SHOW_AS_ACTION_IF_ROOM);
rebuildPasswordLists(); rebuildPasswordLists();
return false; // Query has been handled. Don't trigger default action of SearchView. return false; // Query has been handled. Don't trigger default action of SearchView.
} }
......
...@@ -1232,8 +1232,7 @@ public class SavePasswordsPreferencesTest { ...@@ -1232,8 +1232,7 @@ public class SavePasswordsPreferencesTest {
Espresso.onView(withText(R.string.section_saved_passwords_exceptions)) Espresso.onView(withText(R.string.section_saved_passwords_exceptions))
.check(doesNotExist()); .check(doesNotExist());
Espresso.pressBack(); // Close keyboard. Espresso.onView(withId(R.id.search_close_btn)).perform(click()); // Close search view.
Espresso.pressBack(); // Close search view.
Espresso.onView(withText(R.string.section_saved_passwords_exceptions)).perform(scrollTo()); Espresso.onView(withText(R.string.section_saved_passwords_exceptions)).perform(scrollTo());
InstrumentationRegistry.getInstrumentation().waitForIdleSync(); InstrumentationRegistry.getInstrumentation().waitForIdleSync();
...@@ -1284,34 +1283,7 @@ public class SavePasswordsPreferencesTest { ...@@ -1284,34 +1283,7 @@ public class SavePasswordsPreferencesTest {
Espresso.pressBack(); // Close keyboard. Espresso.pressBack(); // Close keyboard.
InstrumentationRegistry.getInstrumentation().waitForIdleSync(); InstrumentationRegistry.getInstrumentation().waitForIdleSync();
Espresso.onView(withContentDescription("Collapse")).perform(click()); Espresso.onView(withId(R.id.search_close_btn)).perform(click());
Espresso.onView(withText(R.string.passwords_auto_signin_title))
.check(matches(isDisplayed()));
Espresso.onView(withText(startsWith("View and manage"))).check(matches(isDisplayed()));
}
/**
* Check that closing the search via back button brings back all non-password prefs.
*/
@Test
@SmallTest
@Feature({"Preferences"})
@EnableFeatures(ChromeFeatureList.PASSWORD_SEARCH)
public void testSearchBarBackKeyRestoresGeneralPrefs() throws Exception {
setPasswordSourceWithMultipleEntries(GREEK_GODS);
PreferencesTest.startPreferences(InstrumentationRegistry.getInstrumentation(),
SavePasswordsPreferences.class.getName());
Espresso.onView(withSearchMenuIdOrText()).perform(click());
Espresso.onView(withId(R.id.search_src_text))
.perform(click(), typeText("Zeu"), closeSoftKeyboard());
Espresso.onView(withText(R.string.passwords_auto_signin_title)).check(doesNotExist());
Espresso.onView(withText(startsWith("View and manage"))).check(doesNotExist());
Espresso.pressBack(); // Close keyboard.
Espresso.pressBack(); // Close search view.
Espresso.onView(withText(R.string.passwords_auto_signin_title)) Espresso.onView(withText(R.string.passwords_auto_signin_title))
.check(matches(isDisplayed())); .check(matches(isDisplayed()));
...@@ -1373,9 +1345,7 @@ public class SavePasswordsPreferencesTest { ...@@ -1373,9 +1345,7 @@ public class SavePasswordsPreferencesTest {
InstrumentationRegistry.getInstrumentation().waitForIdleSync(); InstrumentationRegistry.getInstrumentation().waitForIdleSync();
// The search bar should still be open and still display the search query. // The search bar should still be open and still display the search query.
waitForView(withId(R.id.search_src_text)); waitForView(allOf(withId(R.id.search_src_text), withText("Zeu")));
Espresso.onView(withId(R.id.search_src_text)).check(matches(isDisplayed()));
Espresso.onView(withId(R.id.search_src_text)).check(matches(withText("Zeu")));
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment