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
Issue #21263 allow typing dynamics from keyboard #22607
base: master
Are you sure you want to change the base?
Issue #21263 allow typing dynamics from keyboard #22607
Conversation
<key>add-piano</key> | ||
<seq>Alt+Shift+P</seq> | ||
</SC> | ||
<SC> |
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.
These 2-key shortcuts should work too:
<SC>
<key>add-pianissimo</key>
<seq>Alt+Shift+P,Alt+Shift+P</seq>
</SC>
<SC>
<SC>
<key>add-mezzo-piano</key>
<seq>Alt+Shift+M,Alt+Shift+P</seq>
</SC>
<SC>
<SC>
<key>add-mezzo-forte</key>
<seq>Alt+Shift+M,Alt+Shift+F</seq>
</SC>
<SC>
<SC>
<key>add-fortissimo</key>
<seq>Alt+Shift+F,Alt+Shift+F</seq>
</SC>
with corresponding commands in the code of course
@@ -25,6 +25,7 @@ | |||
#include "async/notification.h" | |||
#include "notationtypes.h" | |||
#include "types/retval.h" | |||
#include "engraving\types\types.h" |
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.
Causes build faliures
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 correct way would be
#include "engraving\types\types.h" | |
#include "engraving/types/types.h" |
but it looks like you don't need this #include at all, because you use the typedef from notationtypes.h
.
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.
This PR contains a lot of commented or unused code, and it is unclear whether it is supposed to be functional or not. Please remove the unused code and fix the build problem.
|
||
//undoAddElement(dy); | ||
|
||
Articulation* na = Factory::createArticulation(this->dummy()->chord()); |
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.
It seems that you are abusing Articulations to show dynamics. Why not use the Dynamic
class?
@@ -25,6 +25,7 @@ | |||
#include "async/notification.h" | |||
#include "notationtypes.h" | |||
#include "types/retval.h" | |||
#include "engraving\types\types.h" |
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 correct way would be
#include "engraving\types\types.h" | |
#include "engraving/types/types.h" |
but it looks like you don't need this #include at all, because you use the typedef from notationtypes.h
.
Resolves: #21263
In the future I plan to implement this feature exactly as requested. However, I could not implement adding dynamics through text because of limited time and limited understanding. Instead I added support for adding a piano dynamic and adding a forte dynamic with Shift+Alt+P and Shift+Alt+F, respectively