Skip to content

Conversation

@SKefalidis
Copy link
Contributor

@SKefalidis SKefalidis commented Mar 20, 2020

Resolves: https://musescore.org/en/node/275483

It gives the user the ability to change the color of the playback cursor (can't be made fully opaque).
Because having a fully opaque cursor does not have an obvious use if you choose a fully opaque color you get the default transparency level, so the user can change color with even fewer clicks.

To solve the original issue you make the cursor transparent and the voice color black.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [n/a] I created the test (mtest, vtest, script test) to verify the changes I made

//--input state:
PositionCursor* _cursor;
QColor _cursorColor;
const int maxCursorAlpha = 220;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better name: MAX_CURSOR_ALPHA, to immediately see that it is a constant

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@igorkorsukov I chose lower-case because the other constant in the same file is like that. Should I change both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this one is better. Others are best fixed with a separate pull request.

@Spire42
Copy link
Contributor

Spire42 commented Mar 20, 2020

I already have this implemented as part of my redesign of the playhead (some discussion here) in collaboration with @Tantacrul.

My work is currently in progress so there's no PR yet.

@SKefalidis
Copy link
Contributor Author

SKefalidis commented Mar 20, 2020

@Spire42 Should I close this then?

@Spire42
Copy link
Contributor

Spire42 commented Mar 20, 2020

@Spire42 Then I should close this, right?

Yes, I think that would probably be best. Sorry about that.

@SKefalidis
Copy link
Contributor Author

Alright then :^)

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.

3 participants