DlgTrackInfo: add BPM lock toggle to BPM tab - #16025
Conversation
ronso0
left a comment
There was a problem hiding this comment.
Thanks for working on this!
Though it needs a slight rework, see my comments
8faf463 to
0bc0301
Compare
|
Thanks for the review @ronso0! I've addressed all three points:
Also renamed |
0bc0301 to
6f58ca1
Compare
|
Realized I accidentally removed the QSignalBlocker header — it's still used elsewhere in the file. Added it back in the same commit. |
ronso0
left a comment
There was a problem hiding this comment.
Thank you, I left some comments
|
Thanks for the thorough review @ronso0! Addressed all 6 points in the
Let me know if anything else needs changing! |
|
I noticed the lock button no longer turns blue/highlighted Fix would be re-adding |
|
IMO we don't need the button state/highlight. The BPM controls being greyed out and the button text are sufficient. |
ronso0
left a comment
There was a problem hiding this comment.
A few last cleanup requests (sorry, didn't check the ctor and overlooked the other one earlier)
38a5d23 to
028dc25
Compare
|
All review comments addressed: -Remove m_bpmLocked init from ctor since it is set in Thanks for the review, @ronso0! |
|
The CI failure in appstreamcli validate seems unrelated to my changes — it's flagging a 403 on the donate URL and a missing content rating in org.mixxx.Mixxx.metainfo.xml, both of which don't appear to be related to my changes. Is this expected and safe to ignore? |
| // This will only be written back to the track on Apply/OK. | ||
| m_bpmLocked = track.isBpmLocked(); | ||
|
|
||
| tabBPM->setEnabled(true); |
There was a problem hiding this comment.
This is not needed here anymore since we now have updateBpmEditControls() (and wrong if BPM are locked)
There was a problem hiding this comment.
Good catch — removed tabBPM->setEnabled(true) since updateBpmEditControls() already handles that correctly.
|
Looks good otherwise 👍 |
|
Yes, the CI fail is unrelated. The site seems to be n/a temporarily, or probably just requires a more relaxed timeout, idk. |
Thanks for confirming! |
|
Addressed review comments — removed tabBPM->setEnabled(true) since updateBpmEditControls() already handles that correctly, and updated the doc comment to use ///. |
|
Alright, now please squash the commits and we're ready to roll! Btw how much ai did you use for this PR? I just noticed the extensive commit messages and wanna let you know that we don't need those for rather small commits like this one. If anyone is interested, either the commit itself should be clear and if not, they can look up all details/discussion in the PR as that is shown in the git history after merging it. |
Squashing now..
In My inital days I used AI to understand the Codebase and find the right files to edit. |
c346c55 to
8eb0d14
Compare
|
@ronso0 Squashed and force-pushed |
|
@ronso0 did this slip from cache? 😄 |
|
Naa, just other stuff going on. I see you're motivated and eager to get this merged, but priorities shift on a daily basis, so please be patient. |
Of course, take your time! Really appreciate it |
ronso0
left a comment
There was a problem hiding this comment.
Looks good, except the indentation.
Please fix that, then we can merge.
|
Thanks, now squash & force-push please ; ) |
2ff5bc6 to
6af523c
Compare
Squashed and force-pushed! |
Yes, but you also rebased onto main it seems. Which leads to a potentially huge Compare view where the actual PR changes (there should be none when squashing) are hard to spot. Not a big deal for this PR. |
Thanks for the tip! Will research and do better next time ... |
Rebased onto main after BPM lock toggle merge (mixxxdj#16025). Ensures 5/4 and 4/5 scaling buttons respect the BPM lock state, consistent with existing scaling controls.
Summary
Adds a Lock/Unlock BPM button to the BPM tab in the Track Editor dialog (DlgTrackInfo).
Problem
When BPM is locked, the entire BPM tab is disabled with no way to unlock it from within the Track Editor. Users must close the dialog and use the track context menu to unlock BPM.
Changes
QPushButton(bpmLock) to the BPM tab indlgtrackinfo.uislotBpmLockClicked()slot to handle button clicksreloadTrackBeats()to individually disable editing controls instead of the whole tabHow to test
Before Screenshot
After Video
Lock_Unlock.BPM.-.Track.Editor.dialog.mp4