-
-
Notifications
You must be signed in to change notification settings - Fork 401
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
Recreates Emby configuration page with a simpler layout. #4776
base: develop
Are you sure you want to change the base?
Recreates Emby configuration page with a simpler layout. #4776
Conversation
I like it! I was toying around with the Plex settings the other day wondering how I can improve it. I like this approach |
I will resolve the merge conflicts in a few hours. Do you think any other changes are needed? |
3f8f3c9
to
044ba8e
Compare
}; | ||
} | ||
|
||
public processChangeEvent() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like we are recreating some of the built in validation of angular reactive forms here, we should probably use a form with the servers being a form array (it's what we have been moving the other pages to).
What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started off with an angular reactive form but found it easier to implement all the validation and rules this way. I found that I had to listen to every change event anyway to determine if server discovery is required again.
This PR recreates Emby configuration page. All server configuration is now shown in dialogs instead of tabs. Fixes #4424.
I will create another PR for Jellyfin if you are happy with this layout.
New config page view:
Editing existing server:
Creating new server after clicking on "Discover Server" button: