Fix #15779: Add rest section to properties panel
Resolves: #15779
https://user-images.githubusercontent.com/73422868/212297755-5290db19-3e5c-4bdc-8151-a8b43c759b2d.mp4
- [x] I signed the CLA
- [x] The title of the PR describes the problem it addresses
- [x] Each commit's message describes its purpose and effects, and references the issue it resolves
- [x] If changes are extensive, there is a sequence of easily reviewable commits
- [x] The code in the PR follows the coding rules
- [x] There are no unnecessary changes
- [x] The code compiles and runs on my machine, preferably after each commit individually
- [x] I created a unit test or vtest to verify the changes I made (if applicable)
@bkunda Ready for testing :slightly_smiling_face:
Depending on Bradley's opinion about my comment at #15779 (comment), this would mean:
@cbjeukendrup let's go with your option 2 at this stage (to which @Tantacrul has already given his assent).
Pls let me know when it's ready for testing and I'll give it a look.
Pls let me know when it's ready for testing and I'll give it a look.
@bkunda PR is now ready for testing
I have finally had a chance to give this a test, and I think this PR does achieve what it sets out to: namely, incorporating the ‘rest‘ section in Properties with relevant beaming options.
I think there are other issues that should perhaps be solved separately, and agree with @Tantacrul that a better solution would probably be to have a self-contained ‘Beam‘ category that affects both notes and rests (whatever is selected), and perhaps also contains other useful options to, for example, adjust the length of beams that extend over rests (attempting to do this currently results in very strange behaviour), as well an ‘end beam‘ (or ‘break beam right‘) option. As it stands, the behaviour of beam options doesn't always feel easy to intuit, so there can be a lot of moving back and forwards between the beam options panels for rests and notes in an attempt to achieve the desired result. It would ultimately be better to have these options in one place.
I think @oktophonie's observation about not being able to apply beaming options to rests of a quarter note or longer (see comment) reveals a problematic limitation. @iwoithe do let us know if it can be resolved as part of your PR. But I suspect it is perhaps better to raise this as a separate issue.
So with all this in mind, I'd be happy to pass this PR (pending of course a code review @cbjeukendrup) as it does offer some necessary functionality, but I'd then create both an issue to fix the quarter note rest limitation, and a mini-spec to establish a new ‘Beam‘ category that could eventually replace this in a later release.
I'd then create both an issue to fix the quarter note rest limitation, and a mini-spec to establish a new ‘Beam‘ category that could eventually replace this in a later release.
Yeah, raise as separate issues.
Not entirely sure what's causing the quarter note rest limitations. Happy to work on the Beam category when further details are given. :slightly_smiling_face:
I raised the beam-over-quarter issue separately: #16798 @iwoithe - if you'd like to work on it, just leave a comment there