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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Edit admin dashboard menu for plugins #2327

Merged
merged 4 commits into from
Jan 26, 2021

Conversation

MrChip53
Copy link
Contributor

@MrChip53 MrChip53 commented Jan 12, 2021

Changes
I think it would look a little more tidy in the menu if un sorted plugin menu items were under their own section. Also, would like to be able to add plugin items under Live TV(Maybe should allow this for all sections?).

Uhh sorry if its trash code and if you all don't want it thats A-OK 馃槄

Issues

@MrChip53 MrChip53 marked this pull request as draft January 12, 2021 04:17
@MrChip53
Copy link
Contributor Author

No Plugins:

Screenshot (2)

With unsorted and live tv plugins:

Screenshot (3)

@MrChip53 MrChip53 marked this pull request as ready for review January 12, 2021 04:29
src/scripts/libraryMenu.js Outdated Show resolved Hide resolved
@heyhippari
Copy link
Contributor

Some thoughts about UX, which may or may not be valid:

Currently all the config pages are in one place only. It's easy to find them since they all end up in the same place.

Allowing plugins to add themselves to the menu leads to worse UX, imo, since now plugins can end up wherever (I know the feature is already there, but nothing really used it until now).

Furthermore, with the expected plans for the v2 API, plugin pages will be auto-generated from JSON, meaning they'll essentially only be a simple form for configuration. This reduces the utility of this quite a bit, imo.

@MrChip53
Copy link
Contributor Author

MrChip53 commented Jan 22, 2021

Furthermore, with the expected plans for the v2 API, plugin pages will be auto-generated from JSON, meaning they'll essentially only be a simple form for configuration. This reduces the utility of this quite a bit, imo.

This seems like a major regression and severely limits the capabilities of plugins. Can this be an optional feature just used to maybe lower the bar of entry for creating a plugin?

Edit: as a side note, if plugins shouldn't be in the official groups that's fine I can remove all grouping in the menu and just keep the plugin header. Either way it looks a lot cleaner with the plugin header alone imo

@heyhippari
Copy link
Contributor

Can this be an optional feature just used to maybe lower the bar of entry for creating a plugin?

The current way plugin pages work is pretty dirty and unsafe (We're taking HTML wholesale, parsing it all and inserting it into the DOM), and relies pretty much entirely on jellyfin-web polluting window to have access to anything.

The idea is to remove the dependency on web. Ultimately, we want the web client to be "just another client". Currently, the server is heavily tied to it and that's an issue.

@sonarcloud
Copy link

sonarcloud bot commented Jan 24, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@thornbill
Copy link
Member

I don't see any reason to block this right now. It's a marginal improvement to how plugins currently work. When we start planning for a new plugin configuration schema we can decide what features should (or should not) be included.

@thornbill thornbill merged commit 0ff9615 into jellyfin:master Jan 26, 2021
@MrChip53 MrChip53 deleted the library-menu-edit branch February 1, 2021 19:20
@thornbill thornbill added the stable backport Backport into the next stable release label Feb 1, 2021
@thornbill thornbill added this to Active PRs in Release 10.7.0 via automation Feb 1, 2021
joshuaboniface pushed a commit that referenced this pull request Feb 21, 2021
Edit admin dashboard menu for plugins

(cherry picked from commit 0ff9615)
Signed-off-by: Joshua M. Boniface <joshua@boniface.me>
@joshuaboniface joshuaboniface removed the stable backport Backport into the next stable release label Feb 21, 2021
@joshuaboniface joshuaboniface moved this from Active PRs to Completed PRs in Release 10.7.0 Mar 8, 2021
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
Release 10.7.0
  
Completed PRs
Development

Successfully merging this pull request may close these issues.

None yet

5 participants