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

First review on mouseMoveEvent #3

Conversation

IanCaio
Copy link

@IanCaio IanCaio commented Mar 13, 2021

  • Fixes small bug with the vertical selection mode scrolling.
  • Improves logic on the cursor selection of ActionNone.
  • Adds update() + return statements for the actions that didn't have them, and removed the update() from outside the switch.
  • Fixes the code style.

IanCaio added 6 commits March 7, 2021 19:13
	Changes variable names from PianoRoll::mousePressEvent to
camelCase and changes 2 comments.
	Removes a loop that removed every note except one and replaced
it with a clear() call plus appending the note.
	Removes a magic number used for finding the closest note.
	Fixes small bug with the vertical selection mode scrolling.
	Improves logic on the cursor selection of ActionNone.
	Adds update() + return statements for the actions that didn't
have them, and removed the update() from outside the switch.
	Fixes the code style.
@Veratil Veratil merged commit b947627 into Veratil:pianoroll-mouse-refactor Mar 20, 2021
Veratil pushed a commit that referenced this pull request Jun 19, 2021
* Changes mousePressEvent variables to camelCase

	Changes variable names from PianoRoll::mousePressEvent to
camelCase and changes 2 comments.

* Style fixes on mouseDoubleClickEvent

* Removes magic number and costy loop

	Removes a loop that removed every note except one and replaced
it with a clear() call plus appending the note.
	Removes a magic number used for finding the closest note.

* Small fixes + Code style

	Fixes small bug with the vertical selection mode scrolling.
	Improves logic on the cursor selection of ActionNone.
	Adds update() + return statements for the actions that didn't
have them, and removed the update() from outside the switch.
	Fixes the code style.
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