Moved subscription import/export options to (overflow) menu #7458
Conversation
litetex
added
feature request
This comment was marked as resolved.
This comment was marked as resolved.
|
@EvilGremlin Please test this. |
|
Looks good |
|
In your "Import from"/"Export to" screenshot, the arrows aren't visible. I think they should be visible at all times. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
litetex
force-pushed
the
rework-subscription-import-export-ui
branch
2 times, most recently
from
e292816 to
1e15c7b
Compare
|
@TiA4f8R @opusforlife2 |
litetex
force-pushed
the
rework-subscription-import-export-ui
branch
from
1e15c7b to
78cb1ad
Compare
|
@litetex please update the YouTube icon as we decided. Other than that, code looks good to me from a first look. |
|
@Stypox Done :) |
app/src/main/java/org/schabi/newpipe/local/subscription/SubscriptionFragment.kt
Outdated
Show resolved
Hide resolved
* Required for SubscriptionFragment (otherwise the PopUp-menu uses half of the screen) * Size reduction * Fixed/Improved some images: * Bandcamp: Was facing in the wrong direction and used an incorrect logo * Media CCC: Update logo * YT: Added NewPipe logo so that it's not just a rectangle
from code review
litetex
force-pushed
the
rework-subscription-import-export-ui
branch
from
91af97c to
de4b5a8
Compare
| val services = requireContext().resources.getStringArray(R.array.service_list) | ||
| for (serviceName in services) { | ||
| try { | ||
| val service = NewPipe.getService(serviceName) | ||
|
|
There was a problem hiding this comment.
You can use for (service in ServiceList.all()) { and avoid handling strings and avoid the try. The service_list seems to be completely outdated, please remove it altogether (it is not used in any other place):
Sorry for not noticing this earlier
|
Kudos, SonarCloud Quality Gate passed! |
There was a problem hiding this comment.
Code looks good to me. I tested on API 31 importing and exporting things and it works as expected.
The only thing I am still a little bit worried about is discoverability. How can a user know they can import/export subscriptions by tapping on ...? I would replace the "Nothing here but crickets" with a "You can import and export subscriptions from the three-dot menu". But this would be done in a separate PR.
Open an issue until then, so no one forgets it? |
What is it?
Description of the changes in your PR
pngs tosvgsBefore/After Screenshots/Screen Record
Before:
After:
Fixes the following issue(s)
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