Skip to content

Rework playlist#1444

Merged
kambala-decapitator merged 5 commits intoxbmc:masterfrom
wutschel:rework_playlist
Mar 28, 2026
Merged

Rework playlist#1444
kambala-decapitator merged 5 commits intoxbmc:masterfrom
wutschel:rework_playlist

Conversation

@wutschel
Copy link
Copy Markdown
Collaborator

@wutschel wutschel commented Jan 25, 2026

Description

This PR was motivated by reviewing the implementation of editing UITableView. The review showed four main findings:

Unneeded calls of reloadData and selectRowAtIndexPath

Calls of reloadData and selectRowAtIndexPath are not required inside commitEditingStyle and moveRowAtIndexPath:ToIndexPath. They got removed.

Handling removal last playlist

It was also found that after deleting the last playlist item the info label "No items found." was not shown, and that the edit button was still shown active and selected. This is now corrected.

Improper error handling in moveRowAtIndexPath:ToIndexPath

In case an error was reported for Playlist.Remove (e.g. when attempting to move the current playing item) the playlist was not properly reloaded. This resulted in inconsistency between the real playlist in Kodi and the one displayed by the remote app. Also, the wrong item was shown as playing. The correct solution is to call createPlaylistAnimated which will reload the playlist from Kodi and update the UITableView. In case of a (theoretical) issue with Playlist.Insert the same error handling is applied.

Repeated action to remove playlist on server disconnect

In case of server being disconnected a set of actions (fade out playlistTableView, delete playlistData, reloadData, send notification for iPad that playlist is empty) was called repeated, even though playlistData is already empty. An early return is added to serverIsDisconnected to avoid this.

Important: Needs minor rebasing once #1441 and #1442 were merged.

Summary for release notes

Bugfix: Fixes wrong playlist state when facing an error during moving an item in the playlist
Improvement: Shows "No items found." in the playlist once all items were removed by user

@wutschel wutschel force-pushed the rework_playlist branch 2 times, most recently from a9d35e2 to 530151e Compare March 18, 2026 05:57
Copy link
Copy Markdown
Collaborator

@kambala-decapitator kambala-decapitator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I understand, it should be merged after #1442

you may also want to fix typo in the last commit message: dine

@wutschel
Copy link
Copy Markdown
Collaborator Author

as I understand, it should be merged after #1442

Yes.

you may also want to fix typo in the last commit message: dine

Where do you see this?

@kambala-decapitator
Copy link
Copy Markdown
Collaborator

The playlist is already empty, so there is no need to animate / delete / reload / notify anything again. It was already dine.

@wutschel
Copy link
Copy Markdown
Collaborator Author

The playlist is already empty, so there is no need to animate / delete / reload / notify anything again. It was already dine.

Oh my, was looking at the wrong branch. Fixed this, but let's wait until #1442 has been merged.

@wutschel
Copy link
Copy Markdown
Collaborator Author

@kambala-decapitator, the build issue seems to be a problem with the CI job?

@wutschel wutschel force-pushed the rework_playlist branch 3 times, most recently from 8190d6d to 0404571 Compare March 28, 2026 09:24
@kambala-decapitator
Copy link
Copy Markdown
Collaborator

please fix conflicts

…sert

This avoids mismatching playlist in Kodi and remote app after attempting to move the playing item from the app.
The playlist is already empty, so there is no need to animate / delete / reload / notify anything again. It was already done.
@wutschel
Copy link
Copy Markdown
Collaborator Author

Squashed and rebased to master.

@kambala-decapitator kambala-decapitator merged commit 04bb6a7 into xbmc:master Mar 28, 2026
1 check passed
@wutschel wutschel deleted the rework_playlist branch March 28, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants