Feature/search teleport to file - #4713
Conversation
| * Simple holder for a file's parent path and name. Not an android.util.Pair to keep it usable in | ||
| * plain JUnit tests (android.* classes are stubbed out in non-Robolectric unit tests). | ||
| */ | ||
| static class ParentAndFileName { |
There was a problem hiding this comment.
Any reason for not using HybridFile here? Does this work with external searches: SMB, FTP, cloud, etc?
There was a problem hiding this comment.
Any reason for not using HybridFile here? Does this work with external searches: SMB, FTP, cloud, etc?
Thanks for pointing that out! You're right — I was doing naive string splitting on the raw path string, which doesn't correctly handle SMB/FTP/cloud paths.
I’ve updated teleportToFile to use HybridFile directly, so it now correctly handles local, SMB, FTP, and cloud paths.
I also added headless tests covering local, SMB, and FTP paths. The FTP test is skipped on Windows due to a Robolectric file-lock/cleanup issue, but the implementation itself works correctly.
…ch result icon * Update `teleportToFile` in `MainActivity` to accept a `HybridFile` object instead of a string path, leveraging existing methods for parent path and file name retrieval. * Remove manual path-splitting logic and the `ParentAndFileName` helper class. * Update `SearchRecyclerViewAdapter` to pass the `HybridFile` object to `teleportToFile`. * Replace the `ic_location_on_24dp` PNG icon with a new `ic_folder_arrow_right_outline` vector drawable in search results.
Replaces the basic path-splitting unit tests with end-to-end coverage: - Robolectric tests for local, SMB, and FTP teleport paths, plus the no-parent fallback case. - Espresso instrumented test verifying the target file scrolls into view in the real RecyclerView. - ShadowSmbUtil updated to mock parent paths and prevent an NPE during headless SMB file listing. - Reflection-based workaround refreshing TabHandler's cached database reference before each test to avoid a stale SQLite connection left over from Robolectric rebuilding the Application between tests.
|
Hi @EmmanuelMess, just following up on this PR since I addressed your feedback and updated the implementation/tests. Whenever you have time, I’d appreciate another review. Thanks! |
Yes, I'm pretty preoccupied with research right now. If you want a faster review, please provide a comment on how you considered and validated the user experience (especially the back button after teleport), and how you tested on networked files (SMB, etc.). Right now my biggest consideration is how to verify that the PR is thoughtfully implemented. |
scrollToFileName was only cleared when teleportToFile had no resolvable parent, so it stayed set indefinitely otherwise. Any later folder reload would re-trigger a highlight for that file, even on unrelated navigation. Now cleared immediately once consumed in MainFragment, making it a one-shot value. Also moved notifyItemChanged() inside the scrollPosition != -1 check to avoid calling it with an invalid index.
|
Thanks for the detailed feedback — here's a full response covering both points. Back button UX: I traced the back button handling ( Therefore, after a teleport, pressing Back takes the user up one directory level from the location they were teleported to, rather than back to the search results or the previous screen. For example, if you're at Root, search for a file inside While testing this manually, I also found a bug: Networked file testing (SMB/FTP/cloud):
I added headless Robolectric tests covering local, SMB, and FTP path formats in I haven't tested against a live networked server yet, since the change itself is isolated to path handling and |
|
@yasminhosam what have you, the person, done to verify the user interaction? |
2 participants
Description
This PR adds a "teleport to file" icon in search results. Tapping the new icon navigates to the file's parent folder and highlights/selects it there, so the user can act on it directly (rename, move, delete, etc.) without opening it first.
Directory results in search are unaffected — tapping a directory result still opens it directly, as before.
This reuses the existing
scrollToFileName+getScrollPosition()mechanism already present inMainActivity/MainFragment/MainFragmentViewModel, rather than duplicating scroll/highlight logic.Issue tracker
Fixes #4695
Automatic tests
Manual tests
Done
Device: Samsung Galaxy A23
OS: Android 14
Build tasks success
Successfully running following tasks on local:
./gradlew assembledebug./gradlew spotlessCheckGenerative code
This PR used generative code tools (GenAI, LLMs, etc.)
Model: Claude Sonnet 5
Version: 5
Provider: Anthropic