Navigation Menu

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

[CORE] settings refactor #6669

Closed
wants to merge 5 commits into from
Closed

Conversation

mad-max
Copy link
Contributor

@mad-max mad-max commented Mar 9, 2015

Hey guys,
as mentioned and discussed in the forums ( http://forum.kodi.tv/showthread.php?tid=189031 ), I have refactored the settings to a new layout.

I left the old settings windows in the code for backwards compatibility, but still this needs some updates in other skin, as the old windows can be opened but are empty.
I hope this is fine.

ToDo:

  • created new settings windows for media and playback
  • rearranged the order for general settings.xml
  • rearranged platform specific xmls
  • updated strings and skin
  • find a final structure for all settings (this is only c&p from here on)
  • update comments in strings.po to point to new settings location (can be done if there is a final structure)

Here we go with a little preview:

screenshot000

screenshot001

screenshot002

Win32 Testbuild can be found here: https://www.dropbox.com/s/ofak23j4h13jalo/KodiSetup-20150309-3487f9f-settings_refactor.exe?dl=0

Any opinions? There is a need for a final structure ;)
Shall the commits be squashed into one?

@Montellese Thanks for your help!

cheers,
mad-max

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

@MartijnKaijser Saw this also, came from auto conversion in visual studio
Had a fix whitspace grinch commit, so should be fine...
I guess it will be squashed later on anyway

@zag2me
Copy link
Contributor

zag2me commented Mar 9, 2015

That "Media Management" section sticks out a bit due to being 2 words and everything else being 1. Isn't "Media" Enough?

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

Media Management was the term that came up in the forum discussion.
Of course this can be changed. I created the PR as a base for a discussion on the terms and where to put which setting

@fritsch
Copy link
Member

fritsch commented Mar 9, 2015

/me takes Popcorn and opens the bikeshedding pandora:

Media Management might be confusing with actual Sources configuration. What about Media Settings - as those are "settings" after all?

Edit: I have forgotten two words.

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

Again 2 words ;)

@fritsch
Copy link
Member

fritsch commented Mar 9, 2015

What about: Content?

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

To be honest, I like "Media Management", although it is a two word term

@mkortstiege
Copy link
Member

Nah, that´s not Content. It´s more Behavior than anything else. IMO the items have to be re-assigned entirely. Should we open a new forum thread dedicated to this instead of spamming everyone? ;)

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

@mkortstiege There is a forum thread. See first post of this PR

@mkortstiege
Copy link
Member

Sorry, puzzled it with another one.

@jjd-uk
Copy link
Member

jjd-uk commented Mar 9, 2015

Part of the reason for the name Media Management was that as a next step a central "sources" management section for all media types could be added in here, which would probably fit well with Montellese's media import work.

@zag2me
Copy link
Contributor

zag2me commented Mar 9, 2015

Yep its fine, was just commenting that the 1st screenshot looked slightly out of line. Maybe I'm just too obsessed with labels looking similar. I'll bikeshed in the forum thread in future ;)

@MartijnKaijser
Copy link
Member

@zag2me it might look totally different in other languages.

edit:
we should strive to make them as short as possible though because of possible scrolling issues.

@FernetMenta
Copy link
Contributor

it is less clicks on the remote to get to karaoke than to video settings. not ideal :(

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

@FernetMenta
I already thought about rearranging these sections.
What would be the correct layout? At first videos, then music and then other stuff?

BTW: Don't forget that settings level here is expert...karaoke should not be visible in standard

@FernetMenta
Copy link
Contributor

putting video first makes my life easier :)

BTW: that's the point, I always use expert level

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

Yeah, guess video goes first...
I'll wait until we have a final structure and then make the appropriate changes

@mad-max
Copy link
Contributor Author

mad-max commented Mar 9, 2015

@da-anda
Copy link
Member

da-anda commented Mar 10, 2015

IMO reshuffling settings categories only makes sense when we rebuild the entire settings tree, so also regroup settings to new categories etc. I also think we might need a third level because it doesn't make much sense to have "Video library", "Video filelist", "Music library" and "Music filelist" in the media section. I would make more sense and also be cleaner if library and filelist would be grouped in a Video and Music node with the possibility to split up the settings into more subgroups.
So I think we should ignore our current technical limitations and rather think about what structure makes most sense.
Also we should already take upcoming features like retroplayer/games and ADSP into acount and find a nice place for them in the structure. And while reshuffling stuff maybe we should add the ability for addons to extend the settings in the same step (or at least prepare everything for it) so that skinners don't have to redo the settings section over and over again (in case we want addons to inject their settings into the main settings, which might make sense when I think of moving core stuff into addons, like airplay maybe).

@MartijnKaijser
Copy link
Member

we're working in something so this is kinda obsolete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants