Skip to content
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.

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

Move deep search results to SearchView #4042

Merged

Conversation

seelchen
Copy link
Contributor

@seelchen seelchen commented Jan 10, 2024

Description

This PR changes the deep search to show the results in the SearchView and not the MainView anymore. The deep search is refactored to run in a Coroutine instead of an AsyncTask.

Issue tracker

Fixes #4041
Fixes #3357

Automatic tests

  • Added test cases

Manual tests

  • Done

Device:

  • Pixel 7 emulator running Android 13
  • HTC U11 life running Android 10

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembledebug
  • ./gradlew spotlessCheck

@seelchen
Copy link
Contributor Author

I am currently removing all things related to the old search from the MainActivity and MainFragment. During this I saw the retainSearchTask boolean which changes the behavior of MainFragment#goBack to go back to the search (which doesn't seem to work as far as I can tell).

I think that it is helpful to be able get back to the search in general, but I don't know if changing the behavior of goBack is ideal. I think adding an extra button "Go back to search" might be more intuitive.

Since the current implementation does not work with the new deep search in the SearchView, I will delete it. I think that this should be handled in a separate PR.
What are your thoughts about this?

@seelchen seelchen force-pushed the feature/deep-search-integration branch 2 times, most recently from 309393d to 766b6a8 Compare January 10, 2024 23:20
@VishalNehra
Copy link
Member

@seelchen can you please check this codacy issue with your last pr #4020

https://github.com/TeamAmaze/AmazeFileManager/pull/4044/checks?check_run_id=20382785390

@seelchen seelchen force-pushed the feature/deep-search-integration branch 2 times, most recently from 7a0fc58 to e3df339 Compare January 12, 2024 22:35
@seelchen seelchen marked this pull request as ready for review January 12, 2024 23:06
@seelchen seelchen force-pushed the feature/deep-search-integration branch from b522935 to 0d40d19 Compare January 14, 2024 20:14
@seelchen
Copy link
Contributor Author

This is ready for review! I know that it looks like a lot of changes, but most of it is actually just removing an if statement and reducing the indents for the whole block. Since git stores the diff line-based, every line where the indents are reduced are marked as a change...

@seelchen seelchen mentioned this pull request Jan 15, 2024
4 tasks
@VishalNehra
Copy link
Member

Great work! Is it possible to include regex search in filter search and index search? Currently it only works when user does deep search. It leaves user confused. We either need to mention this in settings that regex will only work in deep search or find a way to include regex search in filter and index search.

@seelchen
Copy link
Contributor Author

Great work! Is it possible to include regex search in filter search and index search? Currently it only works when user does deep search. It leaves user confused. We either need to mention this in settings that regex will only work in deep search or find a way to include regex search in filter and index search.

I implemented this in #4050 so that this PR won't become even bigger. If you want, I can completely disable regex search here and it would be readded when #4050 is merged.

@VishalNehra
Copy link
Member

VishalNehra commented Jan 25, 2024

I implemented this in #4050 so that this PR won't become even bigger. If you want, I can completely disable regex search here and it would be readded when #4050 is merged.

Sorry that was an oversight on my end. Amazing work here! This should be good to go :)

@VishalNehra VishalNehra merged commit 12d4990 into TeamAmaze:release/4.0 Jan 25, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show deep search results in SearchView Can search results be sorted by relevance
2 participants