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

[WIP] Fix disable/enable serato metadata checkbox #11416

Closed
wants to merge 5,672 commits into from

Conversation

tech-bash
Copy link

Fix: #11226

daschuer and others added 30 commits November 23, 2022 16:04
This makes navigating key-sorted track lists using a controller much
more convenient. Though as mentioned in the comment, this does not match
the Up/Down cursor key behavior. Should those also wrap around?
This matches the new(ly restored) behavior from
dc83783 by overriding `QTableView`'s
`moveCursor()` function. This way this also works as expected with the
other common selection hotkeys like Shift+Up/Down and Ctrl+Up/Down ->
Space.
The event handler for `WLibraryTableView` now also wraps around the list
when navigating it using the cursor keys. So `[Library],MoveVertical`
can revert to just sending Up/Down key events to the
`WLibraryTableView`.
This happens when the view has not yet been interacted with.
Close incomplete issues after 60 days.
Also add a note to the db schema that positions should be doubles even though the schema was defined as Integer.
Fixes mixxxdj#10993
These functions still use raw pointers so there's a bunch of stuff
that's difficult to catch that can still go wrong, but this at least
removes the need for having to manually deallocate these structures. And
this class didn't adhere to the rule of five, so it was very easy to
misuse it and cause double frees and use after frees.
This needs to be done or else Rubber Band will eat up the initial
transient by introducing a short fade in/2048 sample low-pass.
@JoergAtGithub
Copy link
Member

Your PR is based on the Main branch, but this is a patch for the 2.3 branch. Therefore you see all this differences.
To fix this, you need to rebase your PR: https://github.com/mixxxdj/mixxx/wiki/Using-Git#targeting-another-base-branch

@@ -417,7 +417,7 @@ void DlgPrefLibrary::slotSeratoMetadataExportClicked(bool checked) {
"recommended. Are you sure you want to enable this "
"option?"),
QMessageBox::Yes | QMessageBox::No) == QMessageBox::No) {
checkBox_SeratoMetadataExport->setChecked(false);
checkBox_SeratoMetadataExport->setChecked(true);
Copy link
Member

@ronso0 ronso0 Mar 29, 2023

Choose a reason for hiding this comment

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

You changed the default state and inverted the state if the user explicitely selected that option.
This is not WIP, this doesn't make any sense at all.

The title of #11226 is "Write Serato Metadata checkbox is ignored when writing track metadata is disabled"
= the state of the Serato checkbox is ignored if the checkbox above ("Write track metadata...") is unchecked
Fix: the GUI should reflect the Serato state is irrelevant if "Write track metadata..." is unchecked
= disable (grey out) the Serato checkbox in that case

Copy link
Author

Choose a reason for hiding this comment

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

Ahhh, got it I was thinking the other way around.

Copy link
Author

Choose a reason for hiding this comment

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

I should close this PR and open a new one instead.

@tech-bash tech-bash closed this Mar 30, 2023
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.