-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Add support for advanced content and sort filters in searches #8837
base: dev
Are you sure you want to change the base?
Add support for advanced content and sort filters in searches #8837
Conversation
SonarCloud Quality Gate failed. |
Maybe the UI could be changed from this giant scroll. Ideas? |
@opusforlife2 something like #2251 (comment) would be great in my opinion. |
@evermind-zz Is that UI something you would be interested in implementing? If not, we could always go with the current one as a minimum viable product and improve it later. |
For me the interaction with the current UI is quick and I would like to keep it. But I am willing to give implementing the other UI a shot. Maybe I will like the new UI too or I could make it configurable in settings if someone like me prefer the 'old way' |
first thank you for this... is it possible to have all the options to show up in one box without scrolling... same like the playback speed mene... with a proper layout for both portrait and landscape mode also i think it is important to have [ok] [reset] buttons... so we can choose the filter /sort options in one go... not in multiple times and reloading results in each time |
If we go with the current design for now, I would at least suggest using a dedicated filter icon (via Google's material icons). Having it be stuffed into the hamburger menu isn't very intuitive, IMO. |
@MD77MD I do not really understand what you mean by you have to reload the gui. This version modifies the current menu filter behaviour. Here you first select everything in the menu and than you have to scroll to the top and push search inside the menu. Than the menu will disappear. Have you tested this PR here? But nevertheless I will come up with a dialog style UI |
no matter which UI but a filter icon would be nice |
still working on it! I've rewrote the logic part and have a new UI! But my time is very limited so it will still take some time |
This is the closest thing to what i was trying to convey Portrait mode: landscape mode: |
Also do you think adding #3824 , whether now or later, is possible? |
@evermind-zz, @MD77MD : Are there any news? |
@Angelk90 13 days since the last author reply is not nearly enough time to be asking for updates. This is closer to spam territory. |
I'm still working on it. I'll post an update ASAP |
Edit: comment obsolete. See first post
To choose go to settings -> appearance -> 'Select Search Filter UI' Notes:
The attached apk newpipe-with-search-filters.zip is just a snapshot of my working copy that is bit older than 0.24 release. But I've no time for a rebase atm. In the next days/weeks I will update the code for this PR. Than you can drop me your insights of layouting it better programatically. |
I would love to test it but the apk does not install on Android 11. Can you provide a suitable apk? Normally I can install the artifacts from the workflows, so there must be something wrong with this specific one. |
the problem is that this one has no signature: but with |
It became obvious in just a few tries that the dialog menu is far better to use compared to the action menu. Not only is it comfortably info dense without the need for lots of scrolling, it lets you change multiple settings before applying the filter/sort. |
@evermind-zz : It does not allow to be installed. |
047709d
to
29f0182
Compare
The new 'artifact' one should work. See first post where to find it. |
@evermind-zz : |
@Angelk90 thx! Are screens in portrait mode? |
@evermind-zz : Yes, in portrait mode. |
|
After updating your extractor changes, could you please rebase this PR on the
I don't think this is a good idea to maintain 4 UIs of the search filters, especially with the current app codebase. I personally prefer UIs 1 and 4, with minor changes (like converting them to an alert dialog instead of a dialog similar to the download one if possible and not done in a hacky way). |
I already started the extractor rebase (and almost finished, I just need to integrate 2 things), sorry for not writing that anywhere |
…d content filters SearchFilterLogic.java: ======================= This class handles all the user interaction with the content and sort filters of NewPipeExtractor. The class works standalone to just get the default selected filters eg. during init phase. See in SearchFragment#initializeFilterData() BaseSearchFilterUiGenerator.java: ================================= It extends SearchFilterLogic and is used as a base class to implement the UI interface for content and sort filter dialogs eg. SearchFilterDialogGenerator or SearchFilterOptionMenuAlikeDialogGenerator.
DividerItem was inserted in the content filter framework in the NewPipeExtractor to have a section title for YoutubeMusic. But as UI releated stuff seems a bit out of place in the Extractor I came up with injecting the DividerItem aka section title in the frontend without having to change too much in the frontend.
Dialog looks similar to a action menu based approach but is faster.
This approach is more or less a hack but if all else fails. Could later be dropped or right away.
…ragment The ViewModel that hosts the search filters logic. It facilitates the communication with the SearchFragment* and the SearchFilter*DialogFragment based search filter UI's
…peExtractor's LibraryStringIds class
…g into SearchFragment There is also a configuration option to choose between different search UI's
we can go with UI 1 and 4, but maybe we should decide after rebasing so everyone can have a hands on test. Happy New Year!! |
da7b747
to
4eb71a8
Compare
Quality Gate passedThe SonarCloud Quality Gate passed, but some issues were introduced. 10 New issues |
rebased finished and all seems to work properly. |
Instead of just red, it could be the service, colour, so red for YT, blue for Bandcamp, etc. |
@evermind-zz : You can update, it seems the viewing of videos and channels no longer works. |
I also vote for UI 4. it's the most intuitive one. |
Hi all, I wonder how we should proceed with this PR, since a lot of work has already been put into it, but it seems to have reached a stalemate. Right now the project is refactoring everything to the new Jetbrains Compose UI, and in the process also from Java to Kotlin, and also to a new video player, so I’m afraid none of the code in here is going to apply in a short while. We are also trying to get a designer on board, who could be more authoritative on UX decisions (based on objective research of course). Would it be okay to convert this PR into an issue for the time being, and tackle it again after the rewrite? |
What is it?
Description of the changes in your PR
There are 4 different UI to test
To choose go to settings -> appearance -> 'Select Search Filter UI'
Notes:
Feedback needed
I will then rework the code as there is some duplication right now that should be eliminated firstAny duplication has been removed.Todo
fix sonar findingsfix your findingsstring L10nBefore/After Screenshots/Screen Record
Fixes the following issue(s)
Relies on the following changes
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence