-
-
Notifications
You must be signed in to change notification settings - Fork 3.3k
feat(history): implement infinite search history loading (optimized and without setting) #12728
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
base: refactor
Are you sure you want to change the base?
Conversation
[Bug] Long-pressing Play All-button does nothing
- audioTrackTextView: layout_width=0dp + layout_weight=1 - Make it singleLine with ellipsize="end" - When not fullscreen, hide metadataView so an empty weighted container doesn’t reserve space - Result: controls stay visible on small screens; longer labels can use space on larger screens
Fix Long Audio/Dubs text label puses UI Controls on Player Off Screen in Portrait mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR! Are you working on this as part of the annual ANU comp2120 OSS assignement?
I have a few general comments about this:
- please don't remove copyright comments at the top of file
- loading all items at once will lead to lag when one user has a lot of history. I'd rather paginate the results. I think that the database queries support returning paged
- there seem to be some unrelated commits in your history
- we don't need a setting for this, we can just have infinite loading by default if it's implemented in a non-laggy way, so please remove the setting :-)
| // Use a local copy of NewPipe Extractor by uncommenting the lines below. | ||
| // We assume, that NewPipe and NewPipe Extractor have the same parent directory. | ||
| // If this is not the case, please change the path in includeBuild(). | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your review! Yes, this is part of my ANU OSS assignment. I’ll make the fixes tonight or tomorrow and update the PR soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert this
|
Please use a descriptive PR title so that one can easily see what was done in the PR. |
Added copyright notice and license information to the file.
Comment out the includeBuild section for NewPipe Extractor.
Updated copyright information in the file header.
|
The failing checks are unrelated to my change. |
|
@Jane-Yanlei-Ji The CI failures are indeed related to your changes. Please read the logs carefully.
This image shows a list of checkstyle errors in When building and testing your changes for PR, you should always build the entire project, ensure there are no checkstyle errors and that all tests pass before you hand it over for review. |
Thanks for the feedback! I’ve reverted the comment formatting to match the original style, |
|
@Jane-Yanlei-Ji You still haven't fixed the checkstyle changes and the CI build is still failing. Are you sure you're actually building the code locally? Checkstyle runs as part of the gradle build so you shouldn't even be able to compile it locally without encountering these errors |
My local environment has encountered the same issue. After committing changes, the CI initially shows a pass, but after several hours it fails again. Neither I nor my team members have been able to resolve this problem. We suspect it stems from an issue with our environment configuration. Consequently, I had no choice but to close this PR, re-clone the code, and then complete the feature. |
|
@Jane-Yanlei-Ji I am not sure if you don't know how Checkstyle works, but when you build it locally you will get the Checkstyle errors in the gradle build, and they tell you explicitly how to fix it. I have pulled your PR changes locally and built it and I get the same errors. It is not to do with your environment configuration: when you build it in Android Studio, it runs the You do not need to open another PR to fix this, nor will opening another PR fix this issue if you have the exact same changes. |
Thank you for your reply. I made the changes on the website because my computer encountered some issues that prevented me from completing this project (my computer is incompatible with AGP 8.1.3, as it only supports AGP 8.0.0). Previously, I completed this functionality using another individual's computer. After finishing, I removed the project from their machine. Consequently, without access to a computer, I could only modify some comments and the |
Okay, I see. Why is your computer incompatible with AGP 8.1.3? Do you know? In any case, you do not need to close this PR. All you need to do is find a computer where you can build and run it locally and fix the problems; the PR can remain open in the meantime. Closing it won't serve any benefit. Also, no need to apologise 😅, this is a minor issue. |
Oh,Thank you. I'm still working on resolving this issue. Currently, I knew that the downloaded android-studio-2025.1.4.8-windows.exe installer is incompatible with my computer's system architecture. Consequently, I can only complete this task using another computer. |


What is it?
Description of the changes in your PR
This PR implements Issue #11726, adding a preference option for infinite search history.
Changes include:
Added new preference key infinite_history_enabled in settings_keys.xml
Added string resources in strings.xml
Updated HistoryRecordManager.java to remove the 25-item limit when preference is ON
Updated SearchHistoryFragment.java to respect this setting
Behavior:
When OFF (default): only 25 history items are shown
When ON: users can scroll through all search history
Before/After Screenshots/Screen Record
Fixes the following issue(s)
Relies on the following changes
APK testing
Tested the debug APK built from branch fix-issue-12629.
Confirmed correct display when switch is ON/OFF
No crashes or performance issues found
Due diligence