lmms
lmms copied to clipboard
Adds ablitiy and hotkeys to move controller rack modules up and down
Supersedes #6154 and closes #5773.
from the original PR body.
This commit adds UI changes, signals, and handlers required for moving controllers up and down in a controller rack. The feature was requested in the issue https://github.com/LMMS/lmms/issues/5773. The PR also adds hotkeys for moving controllers and effects in their respective racks, with a combination of ALT + Up/Down.
The implementation is similar to the one used with EffectRacks. Quite small change, good for a first issue to solve (as it was mentioned in the issue itself)
Added move up and move down buttons to context menu Added signals that'll message the requested move Added functions in the ControllerRackView that'll handle those signals Added QActions for the hotkeys to move Controllers & Effects
Tested. Context menu and hotkeys works fine. :+1:
Is this pr based on https://github.com/LMMS/lmms/pull/6154 ?
@zonkmachine yes. I asked the original author and took over the PR because they seem to have lost interest.
I asked the original author and took over the PR because they seem to have lost interest.
I don't see his name in the commit logs. I may have missed that but you should credit the original author in the commit history. I don't know what is the best way to do it but sometimes its just easier to push a bunch of changes to the original PR. Or you can keep the original commit with the original author intact and push it to a new branch with your added stuff to follow. Or you make sure to add a message in the final commit message "Based on original work by: ...".
I actually copy pasted the diff so you won't see his name. So what's the best way to credit him then?
I actually copy pasted the diff so you won't see his name. So what's the best way to credit him then?
I wonder too so I bumped #dev-support.
I did something to try credit @ejaaskel per request from zonk. For some reason, looks like @michaelgregorius is getting some credit too due to a mess up i did with git (for a PR he already wrote).
@Rossmaxx, the best way to keep everything intact would be something along the following lines:
- Add https://github.com/ejaaskel/lmms.git as a remote to your (local) repository.
- Checkout the branch movable-controllers by @ejaaskel.
- Optional: Merge the current master into the branch if necessary.
- Add your changes on top.
- Push the branch to your remote at GitHub.
- Create a new pull request from your remote.
This way GitHub should see all the involved commits and credit @ejaaskel appropriately.
Optional: Merge the current master into the branch if necessary.
And squash it down to one commit probably.
Optional: Merge the current master into the branch if necessary.
And squash it down to one commit probably.
This can also be done during the final merge by using the option "Squash and merge" in GitHub. At least that's what I am using after being told to do so. :sweat_smile:
Noted
@michaelgregorius i fixed it using an interactive rebase.
@michaelgregorius can i get an approval here?
@Rossmaxx, with regards to the functionality everything is working for me. However, it would be nice if there was an indication of what element is currently selected so that the users know which element will be affected by the action.
I think the code might also get simpler if there was a concept of a selected element because some of the signals would not be needed in that case. The shortcuts would be defined for the widget that presents the list and if activated the widget would search for the selected element and work on that. With the current code a list element knows how to move itself (by emitting a signal which is caught by the parent) which is a little bit unintuitive IMO.
However, I guess that's not really in the scope of this PR.
Sorry, forgot about this. I saw a couple of problems with this that I wanted to mention. Give me a second.
Had a feeling I would break build..., sorry, just one second.
I'm going to push my fixes directly here since its easier (and safer) compared to reviewing it on Github again.