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

[gui] merge skin XMLs for MusicPlaylist and VideoPlaylist #8910

Merged
merged 1 commit into from
Jan 26, 2016

Conversation

phil65
Copy link
Contributor

@phil65 phil65 commented Jan 19, 2016

merges the two playlist windows.
One of those had to deal with mixed content before already anyways, so shouldnt be a big deal for skinners to make this change.
@ronie
@mkortstiege

@mkortstiege
Copy link
Member

Fine by me. At some point we might want to merge the actual dupe code of those sections as well though.

@razzeee
Copy link
Member

razzeee commented Jan 19, 2016

Can you sqash this please? I'll try to have a look on the c++ side of things

Edit: NVM, it's not that simple to merge them as they both inherit from video or music specific classes

@ronie
Copy link
Member

ronie commented Jan 19, 2016

tested and works ok.

@akva2
Copy link
Contributor

akva2 commented Jan 19, 2016

@as razzee says it's not entirely trivial. i did try it at some point, holding a reference to a GUIMediaWindow to apply the operations to. iirc i got bitten by protected things and had to friend and at that point i gave up. friend is not an acceptable solution imo.

@phil65
Copy link
Contributor Author

phil65 commented Jan 19, 2016

sorry, @as, we didnt want to disturb you. ;)
@razzeee i will squash when reviewing process is over.

@phil65
Copy link
Contributor Author

phil65 commented Jan 24, 2016

squashed and rebased.

@razzeee razzeee added Component: GUI engine Component: Skin Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v17 Krypton labels Jan 26, 2016
razzeee added a commit that referenced this pull request Jan 26, 2016
[gui] merge skin XMLs for MusicPlaylist and VideoPlaylist
@razzeee razzeee merged commit e66ee42 into xbmc:master Jan 26, 2016
@phil65 phil65 deleted the merge_playlist_windows branch March 1, 2016 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: GUI engine Component: Skin Type: Cleanup non-breaking change which removes non-working or unmaintained functionality v17 Krypton
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants