search

jellyfin-archive / jellyfin-android-original

Android Client for Jellyfin
https://jellyfin.org
GNU General Public License v2.0
271 stars 65 forks source link

Add download function #272

Closed cromefire closed 4 years ago

cromefire commented 4 years ago

The PR is adding a download function that allows downloading of movies/... via android

cromefire commented 4 years ago

The android part should be working, there's just some wire up into web required

JustAMan commented 4 years ago

What makes this a draft?

dkanada commented 4 years ago

@JustAMan it still needs to be hooked up to the web client.

JustAMan commented 4 years ago

That means no one can call this function right now, so what? To make a web hook you need something to call at. I think it should be marked as non-draft and merged before making a web hook.

cromefire commented 4 years ago

Well just add the hook to this PR you should have write permission

nielsvanvelzen commented 4 years ago

This PR will implement a feature request that needs to be updated when the PR is merged / app update is released.

cromefire commented 4 years ago

but there might still be some hookup needed on this site, before it's mergable

dkanada commented 4 years ago

It does still need to be hooked up, but I attempted it already and apparently there's an issue with the dialog, even though the same dialog works fine for the battery prompt.

cromefire commented 4 years ago

What's the error?

dkanada commented 4 years ago

It's complaining that the alert can't be used unless a certain theme is used, but we can't control that with cordova so it would probably be easier to make a settings page and apply it there. I added a method to open a native settings page if you want to remove the dialog and move the network option to a settings activity.

cromefire commented 4 years ago

I'm currently on mobile, I'll look at it tomorrow or so

cromefire commented 4 years ago

It now ready from my side, it should work™

cromefire commented 4 years ago

I've added some translations (en + de for now)

cromefire commented 4 years ago

Okay build is also working now... cordova is a crappy thing...

cromefire commented 4 years ago

Does someone want to merge this?

dkanada commented 4 years ago

@cromefire this will be in the next release for sure, I'm just working out the Azure issues first since that will block a release regardless.

cromefire commented 4 years ago

Yeah just keep in mind that it has to be included in web and has to be tested (which only works after it's in web)

JustAMan commented 4 years ago

I'm going to bite the bullet and merge it, so web part could be cooked.

cromefire commented 4 years ago

Should I open an issue there?

JustAMan commented 4 years ago

Yeah. Make one in jf-web and link to this PR pls.