Skip to content
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

Plugins modified to support es6 migration #240

Merged
merged 11 commits into from
Dec 2, 2020

Conversation

vitorsemeano
Copy link
Contributor

This PR introduces changes to exoplayer and externalplayer plugin, modifying them into classes and expose them in window.

Each plugin will have access to a set of dependencies in its constructor (invoked by jf-web).

also the list of plugins were modified so it can find these plugins in window.

This is still not tested, so i will try to test it somehow with the changes present in jellyfin/jellyfin-web#1994

@nielsvanvelzen
Copy link
Member

I think we should release one version of the Android app before 10.7 releases that supports both 10.6 and 10.7. A month after 10.7 we will remove the 10.6 support.

To achieve this goal I think we need to:

  • Load the .js files from a versioned directory, so we would get something like:
    • /assets/native/10.6/exoplayer.js
    • /assets/native/10.7/exoplayer.js
      This is not ideal because changes would need to made in both files - this is only temporary until we support 10.7>= only
  • When the app starts it will always look up the version of the server it connects to and when the user connects to a server it will always look up the version of the server it connects to
    • Server 10.5<= show error
    • Server 10.6= load 10.6 assets, show toast to user to notify about 10.7
    • Server 10.7>= load 10.7 assets

This way we can keep supporting the 2 most used versions in the next few months.

CC @Maxr1998 what do you think about this plan? And should we do it for 2.2 or for 2.3?

@Maxr1998
Copy link
Member

That sounds good to me. I wouldn't be worried about duplicated files, as we'll later remove those anyways.
Re: which version, I don't really care. Depends on how long it'll take, having another version before this won't hurt though. So probably 2.3?

@nielsvanvelzen
Copy link
Member

If we do it in 2.3 we can confirm Google allows our changes for Android Auto this time. So I think that's a good approach.

@Maxr1998
Copy link
Member

If we do it in 2.3 we can confirm Google allows our changes for Android Auto this time. So I think that's a good approach.

Yep, that's what I was thinking. Let's get that Android Auto out finally!

@nielsvanvelzen
Copy link
Member

Some quick testing for this PR:

@nielsvanvelzen
Copy link
Member

I pushed some fixes that will hopefully load the plugins, need to wait on jellyfin/jellyfin-web#2139 and jellyfin/jellyfin-web#2137 to be merged so I can test though.

@nielsvanvelzen
Copy link
Member

Tested with demo.jellyfin.org/unstable and demo.jellyfin.org/stable for web player, native player and external player and everything seems to work.

The JavaScript in the 10.7 version is formatted with VSCode and I fixed some small issues like duplicate functions and switched from new Promise().... to async functions.

@nielsvanvelzen nielsvanvelzen added this to In progress in v2.2.2 via automation Dec 2, 2020
@Maxr1998 Maxr1998 merged commit 2e6983c into jellyfin:master Dec 2, 2020
v2.2.2 automation moved this from In progress to Done Dec 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants