Skip to content

fix(tv): add playlist deletion confirmation - #121

Merged
CraftWorksMC merged 1 commit into
CraftWorksMC:masterfrom
NathanMartinez:fix/113-tv-playlist-delete-confirmation
Oct 7, 2026
Merged

CraftWorksMC merged 1 commit into
CraftWorksMC:masterfrom
NathanMartinez:fix/113-tv-playlist-delete-confirmation

Conversation

@NathanMartinez

Copy link
Copy Markdown
Contributor

Summary

Fixes #113.

Adds a confirmation dialog before deleting a playlist on Android TV. Previously, long-pressing a playlist would delete it immediately without confirmation.

The dialog uses TV-specific Material components and initially focuses non-interactive content so that a held DPAD_CENTER/OK input does not carry over and accidentally activate one of the dialog actions.

Changes

  • Added a TV-specific playlist deletion confirmation dialog
  • Added Close and Delete Playlist actions
  • Prevented held DPAD_CENTER/OK input from immediately triggering a dialog action
  • Reused the existing playlist deletion flow and string resources

Testing

Tested on:

  • Android TV emulator (Android 14 / API 34)
  • Physical Amazon Fire TV via ADB

Verified:

  • Long-pressing a playlist opens the confirmation dialog
  • Holding DPAD_CENTER/OK does not automatically trigger an action
  • Close dismisses the dialog without deleting the playlist
  • Back dismisses the dialog without deleting the playlist
  • Delete Playlist deletes the selected playlist
  • DPAD focus/navigation works correctly

Screenshots

Confirmation dialog

Screenshot_20261006_172258

DPAD navigation

Screenshot_20261006_172307

After deletion

Screenshot_20261006_172317

@NathanMartinez
NathanMartinez force-pushed the fix/113-tv-playlist-delete-confirmation branch 3 times, most recently from f0faeb2 to 05ff7ce Compare October 7, 2026 01:07
@NathanMartinez

Copy link
Copy Markdown
Contributor Author

@pjonesau Good catch. I missed that the existing TV implementation intentionally no-ops when the playlist ID is null. I’ll preserve that behavior in #121. Thanks!

@CraftWorksMC

Copy link
Copy Markdown
Owner

i think it would be best if you put the dialog into a separate file, PlaylistDialogs.kt maybe, otherwise looks good!

@NathanMartinez

Copy link
Copy Markdown
Contributor Author

@CraftWorksMC

Yeah, I was actually thinking the same thing. I wasn’t sure whether you’d prefer another TV-specific file or to keep the existing dialogs together, so PlaylistDialogs.kt works for me.

Also, since I’ve been working with the TV side a bit more, I’ve noticed a few places where the TV implementation uses different patterns for things like dialogs, focus/input handling, and organization. Would you be open to some small, focused PRs to standardize some of that over time?

I’m also happy to help with testing, CI, contributor docs/templates, and some of the networking work if that’s useful. I have a background in software QA/support/infrastructure, so those are areas I’m pretty comfortable with.

I don’t want to impose a bunch of architectural changes on the project, though, so if you have conventions or a direction you want Chora to follow, I’d be interested in hearing about that. And if there’s a better place than PR threads to discuss broader project stuff, let me know.

@NathanMartinez
NathanMartinez force-pushed the fix/113-tv-playlist-delete-confirmation branch from 05ff7ce to 9f16a8d Compare October 7, 2026 17:53
@NathanMartinez

Copy link
Copy Markdown
Contributor Author

@CraftWorksMC

I updated the PR to include the seperate PlaylistDialogs.kt file and tested it on my device and in emulation via Android Studio.

@CraftWorksMC

Copy link
Copy Markdown
Owner

Awesome!
And for your earlier comment, sorry i didn't see it, if you think there are some parts we can improve on sure go ahead and open PRs for it, depending on how big the change is you could contact me or piripe first explaining what you want to change. for smaller changes like this one it isn't necessary, we appreciate it!

@CraftWorksMC
CraftWorksMC merged commit 18624f9 into CraftWorksMC:master Oct 7, 2026
1 check passed
@NathanMartinez
NathanMartinez deleted the fix/113-tv-playlist-delete-confirmation branch October 7, 2026 20:11
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.

[Bug] Missing playlist deletion confirmation pop-up on Andoid TV (v1.31.0)

2 participants