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

hide Tracks: make removal from playlists and crates optional #14136

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Jan 7, 2025

I propose to make hiding from playlists and crates optional.

As described in #14135 the UX is currently inconsistent and not suitable for my use case.
(I have many short recordings I don't want to see in the Tracks view, but rather have them organized in a 'Samples/rec' crate)
(same may apply to playlists)

Make hiding tracks from playlists optional (they always remain in History) is easy, however, when selecting a playlist it calls PlaylistDAO::removeHiddenTracks(playlistId) which removes them : |
IIUC this function is simply there to have consistent UX with older databases (see quoted comment below).

So according to this comment, that actually needs to be run on all playlists (except History?) but only when upgrading from Mixxx 2.1, because in later versions tracks are removed immediately.

if (!m_keepHiddenTracks) {
// From Mixxx 2.1 we drop tracks that have been explicitly deleted
// in the library (mixxx_deleted = 0) from playlists.
// These invisible tracks, consuming a playlist position number were
// a source user of confusion in the past.
m_pTrackCollectionManager->internalCollection()->getPlaylistDAO().removeHiddenTracks(m_iPlaylistId);

What do you think?

Furthermore, there's an inconsistency Playlists vs. Crates when hiding tracks:
edit figured that out, see first comment

Related bugs:
#10711
#14135

TODO

  • make hiding tracks from playlists optional (they always remain in History)
  • make hiding from crates optional (currently they remain in the crates table)
  • install cleanup when upgrading from Mixxx < 2.1

…earlier

this means that every time tracks are hidden, they are also removed from playlist (if accepted by user)
and there is no need anymore for a per-model/per-feature 'keepHiddenTracks' flag anymore
@ronso0
Copy link
Member Author

ronso0 commented Jan 14, 2025

Furthermore, there's an inconsistency Playlists vs. Crates when hiding tracks:

  • tracks are removed from playlists immediately
  • tracks remain in crates (go to Hidden and select a track that was in a crate -> crate label is made bold)

Ah, okay, this is explained here

// We hide files that have been explicitly deleted in the library
// (mixxx_deleted = 0) from the view.
// They are kept in the database, because we treat crate membership as a
// track property, which persist over a hide / unhide cycle.

Playlists otoh are ordered ...

// From Mixxx 2.1 we drop tracks that have been explicitly deleted
// in the library (mixxx_deleted = 0) from playlists.
// These invisible tracks, consuming a playlist position number were
// a source user of confusion in the past.

which means handling hidden tracks in playlists is complicated, eg. when calculating the # position when displaying playlists, when moving tracks etc.

This means, for my personal use case (have a crate with samples which I'd like to hide from the Tracks view) there's no quick fix in sight.

Not so quick fixes:

  1. have a per-track flag 'showHidden' in the crate_tracks db table
    would allow adding tracks from Hidden to a crate without automatically un-hiding it globally
    Feasible, in conjunction with a "Hide selected track(s) from these crates" dialog it could work.
  2. have a crate property 'showHidden'
    Easy IMO. Assuming my use case (see above) is a niche use case, this might be the way to go.
    Though that also requires to rework the auto-unhide when adding hidden tracks to a crate : |

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.

1 participant