Skip to content

Keep Main Fragment until user's input when searching in settings #12345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Keep Main Fragment until user's input when searching in settings #12345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes in this class should be reverted as they are caused by different indentation settings in the IDE

Original file line number Diff line number Diff line change
Expand Up @@ -764,14 +764,14 @@ private void initSuggestionObserver() {

if (showLocalSuggestions && shallShowRemoteSuggestionsNow) {
return Observable.zip(
getLocalSuggestionsObservable(query, 3),
getRemoteSuggestionsObservable(query),
(local, remote) -> {
remote.removeIf(remoteItem -> local.stream().anyMatch(
localItem -> localItem.equals(remoteItem)));
local.addAll(remote);
return local;
})
getLocalSuggestionsObservable(query, 3),
getRemoteSuggestionsObservable(query),
(local, remote) -> {
remote.removeIf(remoteItem -> local.stream().anyMatch(
localItem -> localItem.equals(remoteItem)));
local.addAll(remote);
return local;
})
.materialize();
} else if (showLocalSuggestions) {
return getLocalSuggestionsObservable(query, 25)
Expand Down Expand Up @@ -876,9 +876,9 @@ public void startLoading(final boolean forceLoad) {
searchDisposable.dispose();
}
searchDisposable = ExtractorHelper.searchFor(serviceId,
searchString,
Arrays.asList(contentFilter),
sortFilter)
searchString,
Arrays.asList(contentFilter),
sortFilter)
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.doOnEvent((searchResult, throwable) -> isLoading.set(false))
Expand All @@ -897,11 +897,11 @@ protected void loadMoreItems() {
searchDisposable.dispose();
}
searchDisposable = ExtractorHelper.getMoreSearchItems(
serviceId,
searchString,
asList(contentFilter),
sortFilter,
nextPage)
serviceId,
searchString,
asList(contentFilter),
sortFilter,
nextPage)
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.doOnEvent((nextItemsResult, throwable) -> isLoading.set(false))
Expand Down
92 changes: 77 additions & 15 deletions app/src/main/java/org/schabi/newpipe/settings/SettingsActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import androidx.preference.PreferenceFragmentCompat;

import com.evernote.android.state.State;
import com.jakewharton.rxbinding4.widget.RxTextView;
import com.livefront.bridge.Bridge;

import org.schabi.newpipe.MainActivity;
Expand All @@ -43,6 +42,10 @@

import java.util.concurrent.TimeUnit;

import io.reactivex.rxjava3.disposables.Disposable;

import com.jakewharton.rxbinding4.widget.RxTextView;

/*
* Created by Christian Schabesberger on 31.08.15.
*
Expand Down Expand Up @@ -86,6 +89,8 @@ public class SettingsActivity extends AppCompatActivity implements
@State
boolean wasSearchActive;

private Disposable searchDisposable;

@Override
protected void onCreate(final Bundle savedInstanceBundle) {
setTheme(ThemeHelper.getSettingsThemeStyle(this));
Expand All @@ -111,9 +116,7 @@ protected void onCreate(final Bundle savedInstanceBundle) {
}
}
} else {
getSupportFragmentManager().beginTransaction()
.replace(R.id.settings_fragment_holder, new MainSettingsFragment())
.commit();
showMainSettingsFragment();
}

if (DeviceUtils.isTv(this)) {
Expand Down Expand Up @@ -189,8 +192,18 @@ private void showSettingsFragment(final Fragment fragment) {
.commit();
}

private void showMainSettingsFragment() {
getSupportFragmentManager().beginTransaction()
.replace(R.id.settings_fragment_holder, new MainSettingsFragment())
.commit();
}


@Override
protected void onDestroy() {
if (searchDisposable != null && !searchDisposable.isDisposed()) {
searchDisposable.dispose();
}
setMenuSearchItem(null);
searchFragment = null;
super.onDestroy();
Expand All @@ -205,16 +218,43 @@ private void initSearch(
final SettingsLayoutBinding settingsLayoutBinding,
final boolean restored
) {

searchContainer =
settingsLayoutBinding.settingsToolbarLayout.toolbar
.findViewById(R.id.toolbar_search_container);

// Configure input field for search
searchEditText = searchContainer.findViewById(R.id.toolbar_search_edit_text);
RxTextView.textChanges(searchEditText)
searchDisposable = RxTextView.textChanges(searchEditText)
// Ignore the initial empty state
.skipInitialValue()
// Wait some time after the last input before actually searching
.debounce(200, TimeUnit.MILLISECONDS)
.subscribe(v -> runOnUiThread(this::onSearchChanged));
.subscribe(charSequence -> runOnUiThread(() -> {
// Change Fragment based on search text
if (charSequence.length() > 0) {
if (searchFragment != null) {
if (!searchFragment.isAdded()) {
getSupportFragmentManager().beginTransaction()
.replace(R.id.settings_fragment_holder, searchFragment,
PreferenceSearchFragment.NAME)
.addToBackStack(PreferenceSearchFragment.NAME)
.commit();
} else {
showSettingsFragment(searchFragment);
}
onSearchChanged();
}
} else {
final Fragment current = getSupportFragmentManager()
.findFragmentById(R.id.settings_fragment_holder);
if (!(current instanceof MainSettingsFragment)) {
getSupportFragmentManager()
.popBackStack(null, FragmentManager.POP_BACK_STACK_INCLUSIVE);
showMainSettingsFragment();
}
}
}));

// Configure clear button
searchContainer.findViewById(R.id.toolbar_search_clear)
Expand Down Expand Up @@ -294,25 +334,36 @@ public void setSearchActive(final boolean active) {
Log.d(TAG, "setSearchActive called active=" + active);
}

// Only reset the text when activating search from the toolbar
if (active && !isSearchActive()) {
resetSearchText();
}

// Ignore if search is already in correct state
if (isSearchActive() == active) {
return;
}

wasSearchActive = active;
if (wasSearchActive) {
searchEditText.post(() -> {
searchEditText.requestFocus();
KeyboardUtil.showKeyboard(this, searchEditText);
});
}

searchContainer.setVisibility(active ? View.VISIBLE : View.GONE);
if (menuSearchItem != null) {
menuSearchItem.setVisible(!active);
}

if (active) {
getSupportFragmentManager()
.beginTransaction()
.add(FRAGMENT_HOLDER_ID, searchFragment, PreferenceSearchFragment.NAME)
.addToBackStack(PreferenceSearchFragment.NAME)
.commit();

if (active && searchText == null) {
showMainSettingsFragment();
} else if (active) {
// Only add if not already added
if (!searchFragment.isAdded()) {
showSettingsFragment(searchFragment);
}
KeyboardUtil.showKeyboard(this, searchEditText);
} else if (searchFragment != null) {
hideSearchFragment();
Expand All @@ -323,8 +374,6 @@ public void setSearchActive(final boolean active) {

KeyboardUtil.hideKeyboard(this, searchEditText);
}

resetSearchText();
}

private void hideSearchFragment() {
Expand Down Expand Up @@ -388,5 +437,18 @@ public void onSearchResultClicked(@NonNull final PreferenceSearchItem result) {
}
}

// Functions needed for testing
protected EditText getSearchEditText() {
return searchEditText;
}

public void setMockEditText(final EditText editText) {
this.searchEditText = editText;
}

protected static int getFragmentHolderId() {
return FRAGMENT_HOLDER_ID;
}

Comment on lines +440 to +452
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to mock these?

//endregion
}
6 changes: 0 additions & 6 deletions app/src/main/java/org/schabi/newpipe/settings/preferencesearch/PreferenceSearcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package org.schabi.newpipe.settings.preferencesearch;

import android.text.TextUtils;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.stream.Collectors;

Expand All @@ -21,9 +18,6 @@ public void add(final List<PreferenceSearchItem> items) {
}

List<PreferenceSearchItem> searchFor(final String keyword) {
if (TextUtils.isEmpty(keyword)) {
return Collections.emptyList();
}

return configuration.getSearcher()
.search(allEntries.stream(), keyword)
Expand Down
1 change: 1 addition & 0 deletions app/src/main/res/layout/settings_preferencesearch_fragment.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
android:layout_height="match_parent"
android:gravity="center"
android:orientation="vertical"
android:clickable="true"
android:visibility="gone"
tools:visibility="gone">

Expand Down
71 changes: 71 additions & 0 deletions app/src/test/java/org/schabi/newpipe/settings/SettingsActivityTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
package org.schabi.newpipe.settings;

import android.widget.EditText;

import androidx.fragment.app.Fragment;
import androidx.fragment.app.FragmentManager;

import org.junit.Before;
import org.junit.Test;
import org.mockito.Mockito;
import org.schabi.newpipe.settings.preferencesearch.PreferenceSearchFragment;

import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.Mockito.when;

public class SettingsActivityTest {

private SettingsActivity activity;
private FragmentManager fragmentManager;

@Before
public void setUp() {
activity = Mockito.spy(new SettingsActivity());
fragmentManager = Mockito.mock(FragmentManager.class);
Mockito.doReturn(fragmentManager).when(activity).getSupportFragmentManager();
// Mock the searchEditText
final EditText editText = Mockito.mock(EditText.class);
activity.setMockEditText(editText);
}

@Test
public void mainSettingsFragmentDisplayedWhenSearchInactive() {
when(fragmentManager.findFragmentById(anyInt()))
.thenReturn(new MainSettingsFragment());

activity.setSearchActive(false);

final Fragment fragment = activity.getSupportFragmentManager()
.findFragmentById(SettingsActivity.getFragmentHolderId());
assertTrue(fragment instanceof MainSettingsFragment);
}

@Test
public void preferenceSearchFragmentDisplayedWhenSearchActiveAndTextNotEmpty() {
when(fragmentManager.findFragmentById(anyInt()))
.thenReturn(new PreferenceSearchFragment());

activity.setSearchActive(true);
Mockito.when(activity.getSearchEditText().getText())
.thenReturn(new android.text.SpannableStringBuilder("query"));

final Fragment fragment = activity.getSupportFragmentManager()
.findFragmentById(SettingsActivity.getFragmentHolderId());
assertTrue(fragment instanceof PreferenceSearchFragment);
}

@Test
public void mainSettingsFragmentDisplayedWhenSearchActiveButTextEmpty() {
when(fragmentManager.findFragmentById(anyInt()))
.thenReturn(new MainSettingsFragment());

activity.setSearchActive(true);
Mockito.when(activity.getSearchEditText().getText())
.thenReturn(new android.text.SpannableStringBuilder(""));

final Fragment fragment = activity.getSupportFragmentManager()
.findFragmentById(SettingsActivity.getFragmentHolderId());
assertTrue(fragment instanceof MainSettingsFragment);
}
}
Loading