-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Fix song-editor regressions and BB-editor bugs introduced in #3487 #4008
Conversation
Fix bugs introduced in LMMS#3487
src/core/Track.cpp
Outdated
Qt::SmoothTransformation ); | ||
new StringPairDrag( QString( "tco_%1" ).arg( | ||
m_tco->getTrack()->type() ), | ||
dataFile.toString(), thumbnail, this ); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, but now you can't select/deselect a single TCO by [CTRL] + Click. That's why I moved this whole block into the mouseMoveEvent(). There we can test if the mouse moved 2 pixels since click(copy TCO) or not(toggleSelected). See: https://github.com/LMMS/lmms/pull/3988/files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BaraMGB Thanks! I'll look into that. Anyway, what's this?
https://github.com/LMMS/lmms/pull/3649/files#diff-cb2a453c979a4777b9330e2b96644f0eL707
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done via 8434d5f.
src/core/Track.cpp
Outdated
@@ -720,16 +720,25 @@ void TrackContentObjectView::mousePressEvent( QMouseEvent * me ) | |||
m_action = ToggleSelected; | |||
} | |||
} | |||
else if( !me->modifiers() ) | |||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BaraMGB Should this be else
, or else if( !me->modifiers() )
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The difference would be you can't move a tco with pressed shift and/or alt key. Perhaps else if(!me->modifier)
is more discriptive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. The latter is consistent with piano roll. Anyway, you changed it to the former in #3649. Was that intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I tried to make the code simpler. But now I think else if(! me->modifiers()
is more discriptive. If someone wants to add a new modifier key, he/she can see where to add it in the code. Also I think, it's better to have a functionality only in a certain circumstance (no modifier key is pressed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay. Done via 9d4c377.
Sorry, next time I ask before I push to your branch. |
@BaraMGB It's okay. That's what I were going to changed, but I've accidentally undone the change before I commit. |
Fix bugs introduced in #3487. Fixes some bugs in #3487, mostly reverting some changes in #3649.
Additional changes:
Supersedes #3988.