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

Bring in ColorChooser.cpp #25

Merged
merged 2 commits into from
Aug 11, 2020
Merged

Bring in ColorChooser.cpp #25

merged 2 commits into from
Aug 11, 2020

Conversation

ryuukumar
Copy link
Owner

No description provided.

IanCaio and others added 2 commits August 10, 2020 17:16
* Starts implementing the feature

The idea of this branch is to allow actions triggered through the context menu of a TCO on the song editor to affect all the selected TCOs. With this commit, only the "Mute/unmute" action affects all selected TCOs, while the others retain their old behavior (only affect the TCO that owns the context menu).

For that, a method was created that processes all actions (the triggered action is parsed as a parameter to the method). In the case of the "Mute" action, it checks if the song editor has selected TCOs, and if it does it mutes/unmutes all of them.

* Allows selected TCOs to be removed too

Now the "Remove" action from the context menu will remove all selected TCOs if there are any.

* Starts implementing selected TCO cut and copy

Now, when multiple TCOs are selected, the context menu actions Cut and Copy will write a DataFile to the clipboard containing the TCO information, so it can later be used to paste it.

The Paste action now checks if there's data in the QApplication clipboard. If there is, it will later paste the TCOs (for now it just prints the data with qWarning). If there's not, it uses the regular TCO paste method that uses the internal LMMS clipboard. Because it now have to decide between the QApplication clipboard and the LMMS internal clipboard, the Clipboard::copy() method now clears anything in the QApplication clipboard, making it empty so LMMS can know it should use the internal clipboard instead in that situation.

Adds safety checks for the selected TCO views.

* Overloads TCW paste selection methods

This commit is a step towards implementing the paste feature of the TCO context menu. It overloads the TrackContentWidget::canPasteSelection and TrackContentWidget::pasteSelection methods so they can be called without having a QDropEvent (only the QMimeData with the copied TCOs information). The overloaded canPasteSelection(MidiTime, QMimeData, bool) method required a third argument which says whether pasting over the same bar should be allowed or not, because it shouldn't when the QDropEvent comes from the same application but should when it doesn't. That is defined in the canPasteSelection(MidiTime, QDropEvent) method.

Also, the pasteSelection(MidiTime, QDropEvent) isn't optimal, since it calls canPasteSelection twice (more details in the comments, but it's basically because the two canPasteSelection methods can return different values depending on the origin of QDropEvent). This could later be fixed by calling canPasteSelection before pasteSelection and removing it from inside the method altogether.

Next step is to add the "tco_" key to the mimeData on the Copy/Cut operations and implementing the paste operation using those methods.

* Adds the TCO type to the clipboard

Adds the key with the TCO type ("tco_" + type number + ":" + TCO Data XML) to the clipboard, so it can be later used by the canPasteSelection and pasteSelection methods.

* Apply changes to "src/tracks/SampleTrack.cpp"

Change the SampleTCOView::contextMenuEvent() method so it behaves the same way as the TrackContentObjectView::contextMenuEvent() method. For that, I had to change the ContextMenuAction enum and the contextMenuAction methods to be protected instead of private, so SampleTCOView can access it.

* Implement the paste action

Now that the canPasteSelection and pasteSelection methods were overloaded, it was possible to implement the paste action using the same logic as the Drag&Drop copy/paste.

Other small changes:
	- Removes the TCO views AFTER creating the TCO data file, instead of before (which probably resulted in an empty data file).
	- Uses the StringPairDrag::mimeType() instead of Clipboard::mimeType() since that's the one the methods from StringPairDrag.cpp recognizes.

* Removes QDebug header

Forgot to remove the QDebug header on the last commit.

* Creates a Context Menu on the TCW for "paste"

Now it's possible to paste a selection of copied TCOs anywhere in the TCW, as long as the rules to paste are met (same rules as Drag&Drop copy/paste). If the rules are not met the "Paste" menu action shows but is disabled.

* Small code refactoring

Saving a few lines of code.

* Avoids double call to canPasteSelection

This commit adds a third parameter to the pasteSelection overloaded method, which will define whether we whould skip the canPasteSelection check. The only situation where we will want to skip it is if we are calling pasteSelection from inside the other pasteSelection method, which will already have checked it. Because of that the default value is false.

Organizes comments as well.

* Separates methods for the actions on selections

Now the remove, copy, cut, paste and toggle mute actions have a separate method for applying them to selections, which are then called from the contextMenuAction method. That made the code more organized and the contextMenuAction method smaller.

Also, the mouse shortcuts for muting and removing (CTRL+middle button, middle button, CTRL+right button) now apply the action on selections as well.

* Fixes small bug and reorganize code

Fixes a small bug where using a mouse shortcut or choosing an action on a TCO that is not selected while other TCOs were selected would result in the selection being affected.

Also reorganizes the code so the conditional for choosing between the selection action and the individual action stays inside the method.

* Move logic to the action methods

Since the methods that called the action+Selection() methods didn't need the list of selectableObjects, I moved it to the inside of the action+Selection() methods and removed the parameter from them.

* Changes logic structure and labels

As suggested by Spekular, the logic structure was changed. Now, the mousePressEvent and contextMenuAction methods determine whether the action should happen to a selection of TCOs or an individual TCO. The list of TCOs to be affected populate a QVector list called "active", which is parsed to the action method that will apply the action to each object in the list.

I changed the method names to removeActive, cutActive, copyActive and toggleMuteActive, since I believe that better describe the behavior. The paste method is still called pasteSelection for now, because the paste behavior isn't related to the active TCOs but to the content of the clipboard.

The contextMenuEvent method on both src/core/Track.cpp and src/tracks/SampleTrack.cpp were changed so they also check if the right-clicked TCO is part of a selection or an individual TCO, and the labels for the actions are changed to better describe the behavior (i.e.: "Delete" for individual TCO and "Delete selection" for multiple TCOs).

* Make removeActive and toggleMuteActive static

removeActive and toggleMuteActive methods are now static so they can be called from anywhere in the code since they don't require a TCO view instance to work. That makes it possible for them to be used in the future if any feature requires this type of action to be called from out of a TCO view instance.

The same couldn't be done for the copyActive and cutActive methods because those require an instance of the TCO view to call them, since when copying to the clipboard some metadata is written using information from the object instance.

* Renamed TCO View paste method

I renamed the TCO View paste method from pasteSelection to just paste, since the TCO view doesn't currently have a paste method (only the TCO class). It's also a name that accurately describes the action: it will paste either a group of TCOVs or a single TCOV on the current TCOV.

I also moved the logic for deciding between the multiple TCOV paste and single TCOV paste inside the paste method.

* Moves repeated code to a new method

This commit adds another method to TrackContentObjectView called getClickedTCOs, which will return a QVector with the TCO views that are supposed to be affected by a context menu action (either the individual right-clicked TCO or the selection of TCOs). Code was updated on the other methods to make use of this new method.

Method names for actions that affect multiple TCOs were changed. Instead of calling them copyActive, cutActive, toggleMuteActive and removeActive, they are just called copy, cut, toggleMute and remove, hence they are overloaded methods for those actions that affect multiple TCOs (their differenciation is the arguments list, which is a QVector list for those).

* Avoid unnecessary calls to getClickedTCOs()

We use a ternary operator inside TrackContentObjectView::mousePressEvent to avoid unnecessary calls to getClickedTCOs when the list is not going to be used.

The contextMenuEvent method on both Track.cpp and SampleTrack.cpp was reformated to use a more appropriate indentation and spacing between method calls.

* Fix indenting in a ternary operator assignment
* Enable mixer color-coding

* Cleanup

* Fix warnings

* Improvements

* Improvements

* Use ColorChooser instead of QColorDialog

* Fix default palette being out of range

* Remove a redundant function

* Rename and make stuff efficient

* Comment on the code

* Make things more efficient

* Fix breaking builds

* Improvements

* Improvements pt. 2

* Improvements pt. 3

* Improvements pt. 4

* Improvements pt. 5

* Apply suggestions from code review

Co-authored-by: Hyunjin Song <[email protected]>

Co-authored-by: Hyunjin Song <[email protected]>
@ryuukumar ryuukumar merged commit 05c42fd into ryuukumar:color-exp Aug 11, 2020
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