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

Mixtrack3: add option for Library navigation mode: Focus | Classic #2

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

ronso0
Copy link

@ronso0 ronso0 commented Jan 20, 2025

This adds controller preferences on top of mixxxdj#14180 to allow users to choose between the focus and classic library navigation.
I think the new behavior is a somewhat 'breaking change', and this is a good minimal example for controller preferences.

Please test if this works:
in the controller settings page there should now be an option for the Library navigation

What do you think?

@endcredits33
Copy link
Owner

Thanks for this. I've tested it and the new option does appear in the controller settings page but changing it doesn't seem to have any impact.

The default focus mode works as expected but after changing it to classic the behaviour remains the same.

@ronso0 ronso0 force-pushed the mixtrack-browse-settings branch from c44e3fa to 722bb2c Compare January 23, 2025 23:21
@ronso0
Copy link
Author

ronso0 commented Jan 23, 2025

@endcredits33
Copy link
Owner

Still not working correctly :( Completely uneducated guess but is it something to do with const libraryModes defining focus & classic as 0 and 1, respectively?

@ronso0 ronso0 force-pushed the mixtrack-browse-settings branch from 722bb2c to 71c9352 Compare January 24, 2025 01:02
@ronso0
Copy link
Author

ronso0 commented Jan 24, 2025

const libraryModes defining focus & classic as 0 and 1, respectively?

No, const just means the array is filled once and can't be altered later on.
Same for const LibraryMode, it's set once from settings.

🤦‍♂️ after all it was a typo...
should now work as expected.

@endcredits33
Copy link
Owner

Ok so the switching between modes works now but the actual mode behaviour isn't quite right.

Focus mode works correctly if I change line 1094 to if (LibraryMode === libraryModes.focus) {

Classic mode still seems to rely on having the right element focused. It's not possible to expand anything in the sidebar with shift+press Browse if the focus is on the track table.

@ronso0
Copy link
Author

ronso0 commented Jan 24, 2025

Okay, good to know it's working.
I suggest you cherry-pick the commit, fix that part and squash that into my commit.

@ronso0
Copy link
Author

ronso0 commented Jan 24, 2025

Yes, 'Classic' mode is limited. Reason I proposed this setting in the first place is that it's useful for those who use Mixxx while having another window in the foreground (Mixxx doesn't have keyboard focus then and the 'new' controls don't apply)
See mixxxdj#11285 and mixxxdj#11285

Shift + press is used for maximizing the library, but it could as well be [Playlist],ToggleSelectedSidebarItem, that's the tradeoff. Another option? ; ) (just kidding..)

@endcredits33 endcredits33 merged commit ec8b021 into endcredits33:2.5 Jan 24, 2025
@endcredits33
Copy link
Owner

endcredits33 commented Jan 24, 2025

Not sure if I did everything correctly (first time using git) but I wasn't able to cherry-pick via the command line due to an error so I merged your commit and then made the fix in a separate commit #a5e443c479504b61a35c6bd544bf63e70feef595

Please let me know if this is incorrect in any way

@ronso0 ronso0 deleted the mixtrack-browse-settings branch January 24, 2025 17:43
@ronso0
Copy link
Author

ronso0 commented Jan 24, 2025

Sure, as long as it works.
Would be best to squash the commits in thebother PR before merge.

What was the error message?
Probably the commit was unknown, you need to add my Mixxx fork to your remote (like you added the Mixxx repo), then fetch so your git knows about all my commits.

@endcredits33
Copy link
Owner

That must be it, I didn't add your fork. It was a fatal error bad object message.

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.

2 participants