mpogue2 / SquareDesk

Fully-featured music player and sequence designer, designed for square dance callers
10 stars 4 forks source link

Bug: Playlist modification actions not working in light mode in V1.0.9 #1149

Open pkbott opened 3 months ago

pkbott commented 3 months ago

Tried adding a playlist in light mode and was not able to at all, no adding/creating/saving/etc.

Switched to dark mode and could add to a playlist column but could not sort the music at all and could only add more to the bottom of the playlist. I have never really used this feature so maybe it's partly me. Went looking for documentation on it in the help section and there isn't much there for me to go on.

Interesting that after creating a playlist in dark mode that it would only work in dark mode too. I have quite a few songs in the playlist in dark mode and when switching to light mode, there is only one song in the playlist.

Needless to say there is a bunch not working here in light mode.

mpogue2 commented 3 months ago

Hmm...I will check it out in light mode...

In dark mode, you can drag to a slot, and it goes in at the bottom. We don't have dragging working yet within a list. Use SHIFT CMD up/down/left/right to move them up/down/to top/to bottom...

pkbott commented 3 months ago

Test loop button missing in light mode now too

mpogue2 commented 3 months ago

@pkbott OK, here's what I found on these two issues:

mpogue2 commented 3 months ago

How weird. I had it failing every time, and now it doesn't fail ever in Light Mode. I added a Track filter in Dark Mode, and now it fails again in Light Mode. This is strange. I have a theory as to what's happening...

mpogue2 commented 3 months ago

My theory was incorrect.

I've gotten it to fail only once more in Light Mode. I'm not sure if it's an intermittent failure, or if something I'm doing is triggering it (rarely).

mpogue2 commented 3 months ago

OK, I MIGHT have a reproducible test case now. Wow this is weird.

If I don't do the Clear Playlist, everything in the context menu works properly on the newly loaded playlist.

BUT, there's a second problem here, probably related: Even when the right-click context menus do work (before I do a Clear Playlist), the keyboard shortcuts NEVER work. They flash the Playlist menu, but they end up doing nothing in Light Mode.

My theory might possibly be right after all: the #ifdef DARKMODE should be replaced with "if (dark mode)" everywhere in the playlist code. I'll try that, now that I have some reproducible test cases to try.

mpogue2 commented 3 months ago

Yes, a test on just PlaylistItemsToTop() suggests that this fixes the context menu problem. It does NOT fix the keyboard shortcuts problem, which appears to be separate.

mpogue2 commented 3 months ago

Note that while Dark Mode playlists can have the same song listed more than once in the playlists, Light Mode playlists cannot. They will only register the LAST song of that name, and the first instance of that song will be missing a number in the # column.

AND, if there's a "hole" in the numbering, context menu in Light Mode will not work properly right around the hole. This is because in prior versions is was not possible to have a song listed twice in a playlist, whereas Dark Mode now allows that.

mpogue2 commented 3 months ago

OK, I needed to fix a problem in the globalEventFilter, too (was this bug there all along?). It wasn't letting CMD-SHIFT-LEFT/RIGHT through to the playlist handling functions, when the focus was in one of the three filter text fields in Light Mode.

I think that's all fixed now, and I tested:

Commit: cdc0c7bc966a8a2709cd78e5c8e51e0a38020ea1

mpogue2 commented 3 months ago

@pkbott I'm gonna turn this comment "Interesting that after creating a playlist in dark mode that it would only work in dark mode too. I have quite a few songs in the playlist in dark mode and when switching to light mode, there is only one song in the playlist." into a new bug, because I don't know how to reproduce this, and I think it's a separate problem.

pkbott commented 3 months ago

Ok, I know that there was a test loop button using light mode on the cue sheet tab since I used that exclusively setting up my patter music but it is now gone. Was this recently changed somehow? I even looked in 1.0.5 and it doesn't show up there either and I know that it did.

mpogue2 commented 3 months ago

@pkbott The Test Loop button is there in all versions in all Light Modes (I went back all the way to 0.9.5), but it's only visible (in all of these versions) when a Patter song is loaded. It's intentionally not visible for singing calls.

So, if you have no song loaded, or if you have a song loaded that is not Patter, then you won't see the button!

If this is the case, but you still need the button (e.g. you have a folder name that contains Patter but is not recognized as such by SquareDesk, e.g. perhaps a singing call is used as Patter), then maybe the algorithm should be different:

"if (Patter) then Test Loop Button = visible" is what we have today, but perhaps

a) "if (not Singing Call) then Test Loop Button = visible", or perhaps even b) Test Loop Button always visible

might be better.

If this is the case, we should open a new Issue because it's probably an Enhancement, rather than a bug related to playlist modification actions.

pkbott commented 3 months ago

no patter test loop button showing and a patter is loaded

Screenshot 2024-07-30 at 7 20 35 AM
mpogue2 commented 3 months ago

@pkbott Cool, thanks! Is this song not in the usual "Patter" folder in the Music Directory?

In your case, if the song title ends in "(Patter)", regardless of which folder it's in (even if a singing call folder), do you consider it to be Patter (like an "override" of the folder that it's in, maybe?)?

Or maybe option c) above is the best approach to handle this (i.e. just leave the button visible all the time)?

In this example below from my setup, the Music tab of Preferences shows both the Music Directory path, and the Music Types (of which "patter" is one).

So, everything in

image

mpogue2 commented 3 months ago

@pkbott I think your other comment belongs attached to this one (re: Test Loop Button)...so, copying it here

"This is my fault. I totally forgot. I took and changed the name of the patter folder on purpose to get the colors to flip between dark and light mode automatically for the text that’s why there’s no more test loop button"

pkbott commented 3 months ago

I personally like the test loop for all but that is me. I do use singing calls as both patter and singers. Currently to make them loop, I was copying them into both patter and singing call folders so the lyrics would show and that I could set up the loop all while in LIGHT mode.

On Tue, Jul 30, 2024 at 5:27 PM Mike Pogue @.***> wrote:

@pkbott https://github.com/pkbott I think your other comment belongs attached to this one (re: Test Loop Button)...so, copying it here

"This is my fault. I totally forgot. I took and changed the name of the patter folder on purpose to get the colors to flip between dark and light mode automatically for the text that’s why there’s no more test loop button"

— Reply to this email directly, view it on GitHub https://github.com/mpogue2/SquareDesk/issues/1149#issuecomment-2259242760, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANJ7Z2TAJXWQ44UH762RCP3ZPAAN3AVCNFSM6AAAAABLPJXIZCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENJZGI2DENZWGA . You are receiving this because you were mentioned.Message ID: @.***>